Raise config error on duplicate process_name within a group - #1726
Open
bardia-key wants to merge 2 commits into
Open
Raise config error on duplicate process_name within a group#1726bardia-key wants to merge 2 commits into
bardia-key wants to merge 2 commits into
Conversation
Previously, when a [group:x] section combined two or more programs
whose process_name expressions expanded to the same value (most
commonly when numprocs > 1 is used and process_name does not include
%(program_name)s), ProcessGroupBase.__init__ silently dropped the
collision: processes for a group are stored in a dict keyed by their
expanded process name, so the later program's process configs simply
overwrote the earlier program's entries in that dict. The result was
that one of the programs never started, with no warning or error
logged anywhere, making the failure very difficult to diagnose.
For example:
[group:a]
programs=b,c
[program:b]
process_name=%(process_num)02d
numprocs=10
[program:c]
process_name=%(process_num)02d
numprocs=10
expected 20 total processes but only started 10, because programs b
and c both expand process_name to "00".. "09" and c's entries silently
replaced b's.
This commit adds duplicate-name detection to
process_groups_from_parser() while building a heterogeneous group's
process list: as each program's processes are added, we track which
program produced each expanded process_name, and raise a ValueError
identifying the colliding process name and both programs as soon as a
collision is found. This matches the existing convention in this file
of raising hard config-parse errors for other invalid process_name/
numprocs combinations (e.g. missing %(process_num)s) rather than
allowing supervisord to start in a broken, partially-functioning
state.
Also updates documentation and the changelog:
- docs/configuration.rst: adds a note under the process_name option
explaining the group-wide uniqueness requirement and the new
startup error, and a matching note under [group:x]'s programs
option, cross-referenced via a new groupx_section label.
- CHANGES.rst: adds an entry under 4.4.0.dev0 describing the bug and
the fix.
Adds three tests to supervisor/tests/test_options.py:
- test_duplicate_process_name_in_heterogeneous_group: reproduces the
numprocs/process_name collision above and asserts the ValueError
mentions the colliding name and both program names.
- test_duplicate_process_name_in_heterogeneous_group_default_process_name:
covers a collision via identical fixed process_name strings (no
numprocs involved).
- test_no_duplicate_process_name_error_when_names_differ: regression
guard confirming process_name expressions that differ per program
(e.g. via %(program_name)s) still produce the full expected process
count with no error.
The previous commit referenced a :ref:`groupx_section` label from the process_name documentation note, but the label itself was never actually added next to the [group:x] Section Values heading, and the :ref: target ended up written as 'group:x' instead. Sphinx's -W build (used in CI) treats this as a fatal 'undefined label' error. Since [group:x] doesn't have a stable anchor to link to, drop the :ref: and just reference the section by its literal name in prose, consistent with how [group:x] is referenced elsewhere in this file. Verified with 'python -m sphinx -b html -W -E docs /tmp/out', which now builds cleanly.
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.
Previously, when a [group:x] section combined two or more programs whose process_name expressions expanded to the same value (most commonly when numprocs > 1 is used and process_name does not include %(program_name)s), ProcessGroupBase.init silently dropped the collision: processes for a group are stored in a dict keyed by their expanded process name, so the later program's process configs simply overwrote the earlier program's entries in that dict. The result was that one of the programs never started, with no warning or error logged anywhere, making the failure very difficult to diagnose.
For example:
expected 20 total processes but only started 10, because programs b and c both expand process_name to "00".. "09" and c's entries silently replaced b's.
This commit adds duplicate-name detection to
process_groups_from_parser() while building a heterogeneous group's process list: as each program's processes are added, we track which program produced each expanded process_name, and raise a ValueError identifying the colliding process name and both programs as soon as a collision is found. This matches the existing convention in this file of raising hard config-parse errors for other invalid process_name/ numprocs combinations (e.g. missing %(process_num)s) rather than allowing supervisord to start in a broken, partially-functioning state.
Also updates documentation and the changelog:
Adds three tests to supervisor/tests/test_options.py: