Skip to content

Draft: Register as / Replacing Modules from other Repositories #57

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

Draft
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

da-h
Copy link
Owner

@da-h da-h commented Dec 30, 2020

Register as / Replacing Modules from other Repositories

This MR presents a proposal for replacing modules from other repositories.

Consider two repositories: main and replace,
where the repository main contains the module hierarchy that plays well together.

This MR:

  • defines any module loading of any module to be repository-internal by default. This means, if a module from the repository main loads another module, i.e. testmodule. The module will always be searched in main, same applies for any other repository.
  • contains a rewrite of register_as to let one module take the place of any other module (also in other repositories).

Previously, a module could only take the place of another module if that module has been called using register_default_module.

New register_as behavior (previously known as redefine_scope):

  • register_as allows any module to be internally renamed to any other module in such a way that the original module will not be loaded anymore
  • this new behavior enables drop-in replacements of modules without changing the original modules in the main repository

@da-h da-h changed the title Register as / Replacing Modules from other Repositories WIP: Register as / Replacing Modules from other Repositories Dec 30, 2020
@da-h da-h changed the title WIP: Register as / Replacing Modules from other Repositories Draft: Register as / Replacing Modules from other Repositories Dec 30, 2020
@da-h da-h added this to the v2.0 milestone Dec 30, 2020
@da-h da-h linked an issue Dec 30, 2020 that may be closed by this pull request
2 tasks
@sbrodehl
Copy link
Collaborator

sbrodehl commented Jan 4, 2021

I like the idea, as I previously run into a few of those issues myself.
Back then, I proposed to mark a whole repository as .lowpriority (see internal ref.), so that (let's say) user modules will take precedence over (let's call them) system ones.

This could be solved by adjusting the order of the paths, where miniflask looks for modules: If one specifies user first, and system later, miniflask will find user modules first, and then the system ones. (Very much like the PATH environment variable in Linux)
Another option would be to assign every module a weight, and a heigher weight takes precedence.

What I don't like is the fact that the user has to know every other module in other repositories and take care of duplicates. What happens if the user uses his repository in another context? Will function calls like register_as fail?
This IMHO also contradicts the idea to loosely couple modules.

@da-h
Copy link
Owner Author

da-h commented Jan 4, 2021

Thank you for your feedback.

I'll first summarize the feature proposals that we talked about in the last weeks and then comment on the pros and cons each.

Module-Name based replacement:
The idea is that module-directories are weighted by their definition order. When the user tries to load a module that exists in multiple repositories, the module with the highest weight gets loaded first.

Pros:

  • the requirement for the user is to know the module name he/she wants to replace (besides the broad functionality)
  • for module import chains a loads b loads c any subset of the modules a, b, c can be replaced by placing the corresponding module as well in the higher-weighted repository

Cons:

  • the user can only define one module that replaces one specific other module, because the module name is the unique identifier that specifies what to load in that case.

Module-ID based replacement:
The user specifies in the module definition the module id of the module to be replaced.

Pros:

  • the requirement for the user is to know the module id he/she wants to replace (besides the broad functionality)
  • for module import chains a loads b loads c is also possible as mf.load behaves in a local-repository-first manner with this PR

Cons:

  • using one single module as a replacement for multiple repositories (or as a standalone module in a third repository) is not directly possible. It would require the module to be replaced to have the same id in both repositories. It is, however easily usable as a standalone module.

Comment:
To allow a module to be replaced as a replacement for multiple repositories, we could add something like:

if mf.is_loaded("main.*"):
    mf.register_as('main.module.to.replace')
elif mf.is_loaded("othermain.*"):
    mf.register_as('othermain.module.to.replace.somewhere.else')

To be honest, I do not see how the first method gives a more loose coupling of modules and requires less knowledge of the repository to replace modules in. In both cases the user needs to know the module to be replaced in a broad manner (like the events it defines) anyway?

I think, both methods achieve more or less the same, however the second method allows multiple modules to serve as replacements for the very same module. Also, the method does not require the module to serve as a replacement to be placed in a separate repository.

@da-h da-h force-pushed the v2-dev branch 2 times, most recently from b98b08f to 260e362 Compare January 4, 2021 10:18
@da-h da-h mentioned this pull request Jan 4, 2021
2 tasks
@da-h da-h marked this pull request as draft January 4, 2021 16:06
@da-h da-h modified the milestones: v2.0, v2.1 Feb 5, 2021
Base automatically changed from v2-dev to master July 26, 2021 16:01
@da-h da-h force-pushed the master branch 8 times, most recently from dfa8411 to ee089c8 Compare July 26, 2021 17:57
@da-h da-h removed this from the v3.0 milestone Oct 6, 2022
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.

rename set_scope and redefine_scope
2 participants