iTop
iTop copied to clipboard
N°4342 - Generic bulk deletion function with memory limit handling
Change function PurgeData in order to delete data by lot
I feel like we should rename the conf parameter to be more explicit about what it does and its usage. It's not a generic buffer size but a "chunk size" or "batch size" for purging data.
Also, why didn't added the conf parameter like the others in the big array instead of a constant?
Discussed during technical review today.
Romain and Anne-Catherine will do a pre-review together. In particular we need to be sure :
- where is this \MetaModel::PurgeData mehod used in iTop ?
- why do we have a memory limit error ? Does the GetColumnAsArray() will load the whole objects besides we only asked for 2 fields (id and finalclass) ?
About the config parameter :
- using an attribute + a getter is the "old way". @accognet please move the declaration to the \Config::$m_aSettings instead
- the parameter shouldn't be visible by default (
show_in_conf_samplearray key) - the param name could be
purge_data.chunk_size? - default value could be 1000
Hi guys, I am one of those who asked for a generic bulk deletion solution a while back, in Combodo's ticket R-032835 (which is linked to ER N°4342). But discovering and reading the content of this PR, while I'm happy something is being done on this front, I realize we might not mean the same :
As far as I understand the code, PurgeData will do only a brutal SQL purge of the object matching the provided filter. But it will NOT guarantee data integrity because it will not do a proper update or delete of related data (update N:N link tables and related objects' history, purge deleted objects' related CMDBChangeOp, block deletions not respecting mandatory link with manual deletion, etc.). As its comments state, "it bypasses standard objects handlers". So it seems this current work would be useless for the use case where the data to be purged has datamodel dependencies (so any instance of class inheriting from cmdbAbstractObject).
What I meant when asking for a generic bulk deletion what to be able to mass delete any instance of classes inheriting cmdbAbstractObject, while respecting all iTop data handeling standards. So it would need to perform all iTop's standard data management to respect data integrity and proper historysation (like any deletionPlan would do). I saw that last version 1.3 of PersonnalDataAnonymizer extension includes some sort of such a batch mass data processing but at the moment I could not find any documentation on how it could be used outside of personal data anonymation for mass delete.
If that's ok with you, I would be interested to discuss this matter with you in any way you would see fit.
Hi @favincen, some of the developers are available on the iTop community Slack team.
Remaining tasks:
- Add a unit test on the method
- Apply suggestions
Great, you can "squash & merge" !