[v2] Implement event handlers registry for built-in plugins#10305
Open
aemous wants to merge 6 commits into
Open
[v2] Implement event handlers registry for built-in plugins#10305aemous wants to merge 6 commits into
aemous wants to merge 6 commits into
Conversation
…a thin initialization function.
…mmand-table operations.
…dlers_registry.py are importable.
aemous
commented
May 13, 2026
Comment on lines
+102
to
+208
| def test_main_command_table_plugins_only_register_against_main(): | ||
| """Plugins listed under building-command-table.main must not register | ||
| against any other events. | ||
|
|
||
| This invariant allows the lazy-loading system to skip importing these | ||
| plugin modules entirely and instead apply pre-computed renames and | ||
| LazyCommand additions from MAIN_COMMAND_TABLE_OPS. If a plugin | ||
| mixes building-command-table.main registrations with other events, | ||
| split it into separate functions: one that only registers against | ||
| building-command-table.main, and another for the remaining events. | ||
| """ | ||
| main_entries = PLUGIN_REGISTRY.get('building-command-table.main', []) | ||
| violations = [] | ||
| for module_path, fn_name in main_entries: | ||
| emitter = _AuditEmitter() | ||
| mod = importlib.import_module(module_path) | ||
| fn = getattr(mod, fn_name) | ||
| fn(emitter) | ||
| non_main = [ | ||
| e | ||
| for e in emitter.registrations | ||
| if e != 'building-command-table.main' | ||
| ] | ||
| if non_main: | ||
| violations.append( | ||
| f'{module_path}.{fn_name} also registers against: ' | ||
| f'{non_main}' | ||
| ) | ||
| assert not violations, ( | ||
| 'The following building-command-table.main plugins register ' | ||
| 'against additional events. Split each into separate functions ' | ||
| 'so that the building-command-table.main function only registers ' | ||
| 'against that single event:\n' | ||
| + '\n'.join(f' - {v}' for v in violations) | ||
| ) | ||
|
|
||
|
|
||
| def test_main_command_table_callbacks_only_add_or_rename(): | ||
| """Callbacks registered against building-command-table.main must only | ||
| add new commands or rename existing ones. | ||
|
|
||
| MAIN_COMMAND_TABLE_OPS replaces these callbacks at runtime with | ||
| LazyCommand additions and direct renames. If a callback also | ||
| modifies existing command table entries (e.g. changes properties on | ||
| a command object), that modification would be silently lost. | ||
| """ | ||
| session = botocore.session.Session() | ||
| services = session.get_available_services() | ||
|
|
||
| main_entries = PLUGIN_REGISTRY.get('building-command-table.main', []) | ||
| violations = [] | ||
|
|
||
| for module_path, fn_name in main_entries: | ||
| collector = _CallbackCollector('building-command-table.main') | ||
| mod = importlib.import_module(module_path) | ||
| fn = getattr(mod, fn_name) | ||
| fn(collector) | ||
|
|
||
| for callback in collector.callbacks: | ||
| cb_name = f'{callback.__module__}.{callback.__qualname__}' | ||
|
|
||
| # Build a fresh command table for each callback. | ||
| class _Placeholder: | ||
| def __init__(self, name): | ||
| self.name = name | ||
|
|
||
| command_table = OrderedDict() | ||
| for svc in services: | ||
| command_table[svc] = _Placeholder(svc) | ||
|
|
||
| snap_id_to_key = {id(v): k for k, v in command_table.items()} | ||
| snap_id_to_name = {id(v): v.name for k, v in command_table.items()} | ||
|
|
||
| callback(command_table=command_table, session=session) | ||
|
|
||
| # Classify every change. | ||
| new_id_to_key = {id(v): k for k, v in command_table.items()} | ||
| renamed_ids = set() | ||
|
|
||
| # Detect renames. | ||
| for obj_id, new_key in new_id_to_key.items(): | ||
| old_key = snap_id_to_key.get(obj_id) | ||
| if old_key is not None and old_key != new_key: | ||
| renamed_ids.add(obj_id) | ||
|
|
||
| # Detect modifications: an existing (non-renamed) entry whose | ||
| # .name property changed, or any entry that was removed. | ||
| for obj_id, old_key in snap_id_to_key.items(): | ||
| if obj_id in renamed_ids: | ||
| continue | ||
| if obj_id not in new_id_to_key: | ||
| violations.append(f'{cb_name} removed command {old_key!r}') | ||
| continue | ||
| new_key = new_id_to_key[obj_id] | ||
| cmd = command_table[new_key] | ||
| if cmd.name != snap_id_to_name[obj_id]: | ||
| violations.append( | ||
| f'{cb_name} modified .name on {new_key!r} ' | ||
| f'without renaming' | ||
| ) | ||
|
|
||
| assert not violations, ( | ||
| 'Callbacks registered against building-command-table.main must ' | ||
| 'only add or rename commands. The following callbacks perform ' | ||
| 'other modifications that would be lost when replaced by ' | ||
| 'MAIN_COMMAND_TABLE_OPS:\n' + '\n'.join(f' - {v}' for v in violations) | ||
| ) |
Contributor
Author
There was a problem hiding this comment.
Note for reviewer: these two tests add some invariants about plugins/functions that register against building-command-table.main that are needed for the initial migration from the monolith to the registry. In a future PR, we will remove the init functions that add/rename commands entirely, and we will delete these two tests.
These two tests are just asserting invariants of the current state that is needed for the script to work correctly.
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
1 participant
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.
Issue #, if available:
Notes:
awscli/handlers.pyto an alternative registry format inawscli/handlers_registry.py, which will be used in a future PR to efficiently initialize plugins only when they are needed. The registry will be used to determine which plugins are needed during event-emission and command-execution. In a future PR, the monolithicawscli/handlers.pywill be removed and replaced entirely byawscli/handlers_registry.py.Description of changes:
awscli/handlers_registry.pythat maps events to plugins that register against them. It also declaratively models plugins that register againstbuilding-command-table.mainto add or rename commands in the command table. The registry was automatically generated from a script that probes the existing initialization functions. The script can be reviewed here: generate-plugin-registry.tar.gz.awscli/handlers.pyin a thin initialization function. The initialization function gets called inawscli/handlers.pyinstead of registering against the events inawscli/handlers.pydirectly.awscli/handlers.pyto the newawscli/handlers_registry.py. Also, it is more consistent with the majority of existing plugin-initialization inawscli/handlers.py.building-command-table.mainto add or rename commands, that also register against at least 1 other event, these functions were split into two functions: one that registers the add/rename callbacks, and one that registers against everything else.awscli/handlers_registry.pyare importable, and misc. tests verifying expected behavior of new components.Description of tests:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.