Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ambiguous register_default_module order #102

Merged
merged 2 commits into from
Nov 21, 2021

Conversation

da-h
Copy link
Owner

@da-h da-h commented Nov 21, 2021

Fix ambiguous register_default_module order

Attention: This is a breaking change.

This MR implements a policy on the order of multiple realized register_default_module-calls.

Description

Imagine, you register multiple default-modules: Each such calls checks for an event, and, if not registered, loads a given module and may overwrite its settings.

Setup:

mf.register_default_module("module1", required_event="event1", overwrite_globals={
   "var1": 42
})

mf.register_default_module("module2", required_event="event1", overwrite_globals={
   "var1": 1337
})

mf.register_default_module("module1", required_event="event1", overwrite_globals={
   "var1": 1234
})

mf.register_default_module("module2", required_event="event1", overwrite_globals={
   "var1": 4321
})

Problem:
In case multiple such calls ask for the same event, the order has been unspecified until now.
In more detail, there are in-fact distinct orders of actions that need to be define in order to use this feature consistently.

  1. Should we use the oldest or the newest command to get the action to be done?
  2. Should we ignore or use the other definitions, if they are fulfilled as well. I.e. in the example above, we have two calls of module1. Which settings should be used in case module1 has already been loaded?

Previously:

  • The first call was checked first. If the event has not been found, the given module will be loaded and the settings will be overwritten. (In our example module1 will be loaded).
  • The check proceeds with the next call. In case it has been fulfilled, the previous-settings state was overwritten again. In case another module is given, the settings were ignored. (The variable var1 would get the value 1234).

New Behavior:
The previous solution is counter-intuitive, as a later call does not "overwrite" the choice of modules.
(For instance, imagine several such commands in a dependency chain. The most-recent call such define the module to be loaded.)
Thus, this MR changes the order in the following way:

  • The most recent module-calls are used to actually choose which module to load in this case. (In our example it is module2).
  • In the list of all such calls with the same default-module, all settings are used, but each given setting-list overrides any previous setting. (In our example, var1 would get the value 4321).

Check all before creating this PR:

  • Documentation adapted

@da-h da-h merged commit 8fd2c2d into master Nov 21, 2021
github-actions bot pushed a commit that referenced this pull request Nov 21, 2021
Fix ambiguous register_default_module order
@sbrodehl sbrodehl deleted the fix_register_default_module_order branch October 7, 2022 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant