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

Add Reserved contract #110

Open
jakub-borusewicz opened this issue Oct 23, 2021 · 3 comments
Open

Add Reserved contract #110

jakub-borusewicz opened this issue Oct 23, 2021 · 3 comments

Comments

@jakub-borusewicz
Copy link

I think there could be one more very generic contract type - reserved contract. It would be basically a mirror reflection of forbidden contract. Basic configuration could look like this:

Configuration options:
        - allowed_modules: A list of modules that are allowed to import the reserved modules.
        - reserved_modules: A list of modules that can be imported by the source modules, but not any other.

This contract would fail if any of reserved_modules are imported OUTSIDE of allowed_modules. This would be very helpful to enforce DDD or ports and adapters approach, when you want to prevent leaking implementation details bound to specific third-party library/API to rest of the app.

PS - great library, congratulations!

@seddonym
Copy link
Owner

seddonym commented Nov 1, 2021

Thanks for the suggestion and sorry for the delay, I've been on holiday.

It's an interesting idea. Is something like this what you are envisaging?

[importlinter:contract:1]
name = Restrict importing of sqlalchemy
type = reserved
reserved_modules = sqlalchemy
allowed_modules = mypackage.repository

Alternatives

It's worth exploring design alternatives, do you have any thoughts on these?

Forbidden + ignored import

An alternative approach would be to use a forbidden contract together with ignore_imports:

[importlinter:contract:1]
name = Restrict importing of sqlalchemy
type = forbidden
source_modules =
    mypackage
forbidden_modules =
    sqlalchemy
ignore_imports =
    mypackage.repository -> sqlalchemy

Pro: It already works.
Con: I like to think of ignore_imports as representing tech debt: imports that are undermining the architecture. Here, we have to use it to express the intended architecture instead, which is a shame.

Verbose forbidden

[importlinter:contract:1]
name = Restrict importing of sqlalchemy
type = forbidden
source_modules =
    # List all the other packages except mypackage.repository
    mypackage.foo
    mypackage.bar
    mypackage.baz
forbidden_modules =
    sqlalchemy

Pro: It already works.
Con: It requires verbose listing of all the other packages, and if new packages are introduced then they won't get checked.

Closed layers

A common architectural pattern that isn't supported yet is closed layers. Maybe we could add an option to the layers contract like so, and treat the reserved module as a lower-level layer.

[importlinter:contract:1]
name = Restrict importing of sqlalchemy
type = layers
closed = true  # N.B. this isn't supported yet.
layers =
    mypackage.foo
    mypackage.bar
    mypackage.baz
    mypackage.repository
    sqlalchemy

Pro: fits well with an existing architectural pattern, and is a feature that I'd like to add at some point anyway.
Cons: less flexible; requires additional development; requires all packages to be listed as layers, and if new ones are introduced they won't get checked.

Protected

This is the same as the reserved idea but with different language.

[importlinter:contract:1]
name = Protect importing of sqlalchemy
type = protected
protected_modules = sqlalchemy
source_modules = mypackage.repository

Pro: simple, flexible, IMO easier to understand than the 'reserved' concept.
Cons: requires development.

@jakub-borusewicz
Copy link
Author

Yeah, Verbose forbidden is what I initially wanted to avoid, for reasons you mentioned. I didn't think about Forbidden + ignored import, yeah, seems it would solve this issue, but as you mentioned, it isn't quite elegant. Adding a separate Protected/Reserved contract would be the best option IMHO. Actually, I've tried to implement it on my own, but I didn't succeed with the first attempt. Maybe I'll make a PR when I'll have some spare time, in meantime I wanted to let you know the idea in case you would have come up with simple implementation for this :)

@seddonym
Copy link
Owner

seddonym commented Nov 2, 2021

I agree, I think the Protected contract would be the best way forward.

Happy to consider a PR at some point - in the meantime you can always make a custom contract to do this.

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

No branches or pull requests

2 participants