Skip to content

Give anonymizer metadata responsibility to the AnonymizerRegistry class #221

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

Merged
merged 1 commit into from
Jul 21, 2025

Conversation

pounard
Copy link
Member

@pounard pounard commented Jul 18, 2025

Preparation work for later, which will allow dynamic anonymizers to be created differently than from a class name. This allows dynamic anonymizers creation from configuration.

This will allow packs to be written without PHP code.

In order to allow this:

  1. we move all anonymizer metadata code into the registry itself, it's not the anonymizer responsability anymore (since some will be dynamic, we cannot know them beforehand in order to create them then ask for metadata, this is a chicken and egg problem),
  2. we move the responsibility to create instances to the registry as well, which then also acts as a factory.

This is a first step, tests won't pass right away since I didn't fix them all, it's a PoC for discussion.

@pounard pounard requested a review from SimonMellerin July 18, 2025 08:50
@pounard pounard force-pushed the datalist-1-metadata branch from 0f49c44 to c64c47b Compare July 18, 2025 11:08
Copy link
Member

@SimonMellerin SimonMellerin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

except my 2 comments, seems good to me.

Just one question, are you sure that if we use the same anonymization twice (on 2 different tables and/or columns, it is instantiated only once ? (ie one sample table will be created and used)

@pounard pounard force-pushed the datalist-1-metadata branch from 16fbefa to 5538b7d Compare July 18, 2025 13:36
@pounard
Copy link
Member Author

pounard commented Jul 18, 2025

@SimonMellerin

Just one question, are you sure that if we use the same anonymization twice (on 2 different tables and/or columns, it is instantiated only once ? (ie one sample table will be created and used)

Yes, a new instance is being created every time, if not, then there is a bug, but not in this code, probably more in the Anonymizator, but we would already have found it in that case.

Copy link
Member

@SimonMellerin SimonMellerin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok for me, if we put aside the fact that the tests don't pass 😅

@pounard
Copy link
Member Author

pounard commented Jul 18, 2025

ok for me, if we put aside the fact that the tests don't pass 😅

Yolo, the only way of living.

@pounard pounard force-pushed the datalist-1-metadata branch from 5538b7d to 8cd8ce8 Compare July 21, 2025 09:44
@pounard pounard force-pushed the datalist-1-metadata branch from 8cd8ce8 to 03f8fe7 Compare July 21, 2025 09:49
@pounard pounard merged commit e53ca69 into main Jul 21, 2025
30 checks passed
@pounard pounard deleted the datalist-1-metadata branch July 21, 2025 10:02
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.

3 participants