seddonym / import-linter

Import Linter allows you to define and enforce rules for the internal and external imports within your Python project.
https://import-linter.readthedocs.io/
BSD 2-Clause "Simplified" License
664 stars 45 forks source link

Pass contract key to contract on instantiation #228

Closed exterm closed 1 month ago

exterm commented 1 month ago

It would be great to be able to use the contract key as part of error output on custom contract types. Since the key is specific to the contract, not the contract type, we need to pass it in.

The use case for this is that we have added a ratcheting mechanism on some custom contract types so that we can work off a list of violations. We tell users to record new violations in case of urgent changes, in which case we'd like them to run import linter scoped to just the affected contract. So, the contract key would make a lot of sense in the error message.

But the contract key is currently not available within the contract instance.

exterm commented 1 month ago

Ignore me, it's actually in self.contract_options['id'].

seddonym commented 1 month ago

No problem. Incidentally are you aware there is a Python API for reading the configuration. I've used this myself for ratchet mechanisms. Not sure it will help with your specific use case though.

exterm commented 1 month ago

I was not aware, but this is cool. Can you add a little more detail on how you used it for a ratcheting mechanism? I can see using it for user feedback messages, anything else I should be thinking of?

seddonym commented 1 month ago

We have a test that calls the Python API to get the contract we're interested in, and then asserts that the length of the ignored_imports list larger than a hardcoded MAX_IGNORED_IMPORTS constant. If you want you could assert that it's equal to a constant instead, that would encourage people to update that number when they remove an import.

exterm commented 1 month ago

ah, we actually have a more specific ratcheting mechanism that keeps a list of all imports that violate the contract.

So if you remove one violating import, add another, you'll still get a contract failure, even though the total number is the same.

This is similar to how https://github.com/Shopify/packwerk works, which I co-authored.

I guess we could keep that list in ignored_imports but our lists easily have hundreds of elements and seem misplaced in a configuration file.

seddonym commented 1 month ago

So if you remove one violating import, add another, you'll still get a contract failure, even though the total number is the same.

I guess you could hash the contents of ignored_imports as well, to make sure it doesn't change? Or compare it against a known good list?

Ultimately the contract could be used as a ratchet in itself - it just depends on how on board reviewers of the pull requests are in stopping people adding to the ignored imports! At work, I ended up putting in the secondary tests because I found that in practice, people added to the list (several hundred devs work on the code base). With a few more hoops to jump through it seemed to stop it happening, even though the devs could just change the numbers in the test.