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

Refactor the anonymisation Config class #181

Open
martinburchell opened this issue Dec 20, 2024 · 2 comments
Open

Refactor the anonymisation Config class #181

martinburchell opened this issue Dec 20, 2024 · 2 comments

Comments

@martinburchell
Copy link
Collaborator

I propose a design improvement to separate the anonymisation Config object from its creation from the configuration file. It would more efficient to only create the Config when it is needed and remove the need for the current singleton class. Currently the tests that involve anything to do with the configuration end up mocking out a lot of unwanted behaviour. It would be good to be able to create an arbitrary Config with only the minimum fields set for a particular test and remove the need for the mock and RUNNING_WITHOUT_CONFIG switches.

The secret mapping tables in anonymise/models.py rely on the singleton class because their column types depend on the configuration. We could change these to be created programmatically. @RudolfCardinal looking at the StackOverflow links at the top of this file, it looks like you've looked into this already. Are you aware of any difficulties with this approach that I've overlooked?

@RudolfCardinal
Copy link
Collaborator

It's an excellent idea. The singletons are a bit awful (likewise with Django). I expect I tried ineffectually and failed. But I wonder if at least part of the problem was that SQLAlchemy ORM classes inherited from a Base class that is defined around a specific MetaData object, making on-the-fly changes awkward. I think that SQLAlchemy 2 fixes this, keeping the abstraction more abstract, but I've not grasped everything in full yet - but perhaps we should do #179 first (I've started working on that) and that might simplify this one.

@martinburchell
Copy link
Collaborator Author

Yes I'd be happy to wait until SQL Alchemy 2.0 and I'll take full responsibility for my convoluted test code in the mean time!

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