Fix/issue 416 start client name#536
Merged
Merged
Conversation
start_client(name: X) and child_spec(name: X) were documented but ignored
:name. start_client/1 called Client.start_link/1 (single arg) which used
the implicit __MODULE__ default; child_spec/1 used :name only as the
supervisor :id while the start MFA still resolved to the single-arg form.
A second call collided with :already_started, and :id vs registered name
diverged uselessly.
API changes:
- KafkaEx.API.start_client/1 extracts :name from opts and passes it as
the second arg to Client.start_link/2. Defaults to :no_name (unnamed)
when :name is absent — eliminates the implicit-global-registration
footgun. :name accepts atom, {:global, term}, or {:via, Module, term}.
- KafkaEx.API.child_spec/1 threads :name into the start MFA tuple so
the spawned child registers under the requested name.
- KafkaEx.Client.start_link/2 @SPEC widened from atom to
GenServer.name() | :no_name — the runtime already accepted any of
these shapes via GenServer.start_link/3's name: option.
State cleanup:
- Drop the vestigial state.worker_name field. It was set once in init/1
and read only by a Logger.debug in terminate/2. Honors the original
reporter's "the name of the worker and genserver should never cross
paths" by removing the redundant identity. State.static_init/2 →
static_init/1.
- terminate/2 log now reports the registered name (via
Process.info(self(), :registered_name)) for named clients, falling
back to self() for unnamed.
Test coverage (test/integration/lifecycle/start_client_test.exs, 8 tests
+ doctest):
- default no-:name returns unnamed pid; Process.whereis(KafkaEx.Client)
is nil
- name: nil equivalent to omitting :name
- atom name registers locally
- two clients with distinct atom names coexist
- name: {:global, term} registers globally
- name: {:via, Registry, {reg, key}} works
- child_spec/1 produces the expected :id + start MFA shape
- supervised children with distinct :name run independently; killing
one triggers supervisor restart under the same registered name, and
the restarted client serves API.metadata/1 cleanly
Refs #416.
- CHANGELOG.md: new "## Unreleased" section with a Breaking Changes
bullet describing the start_client() unnamed-by-default behavior
change and pointing at UPGRADING.md for migration.
- UPGRADING.md: new "Breaking Changes" subsection with old/new code
examples and the recommended migration (pass name: KafkaEx.Client
explicitly to preserve the prior implicit registration).
- README.md, usage-rules.md: the documented supervision child spec
{KafkaEx.Client, name: MyApp.X, ...} did not actually honor :name
(KafkaEx.Client has no child_spec/1 extracting :name; it falls
through to start_link/1 with the implicit default). Switch both
docs to {KafkaEx.API, name: ..., ...}, which now honors :name
correctly via the fix in this PR's implementation commit.
Refs #416.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.