metacpan-api
metacpan-api copied to clipboard
Verify Indices Mappings and Clear all Indices
This implements the functionality discussed at: Fix Corrupted Indices and Verify Mappings It is also able to fix difficulties and corruptions as documented at Unexpected Index Corruption
Additionally it implements Exit with Error Code for unconfirmed Operations.
To test all the functionality it features several Mapping Script Tests.
This is a big (adding 700 lines) change that introduces valuable functionality. My only observation is it seems splittable conceptually into two pieces of functionality; the verify, and the clear-indices. Would it make sense to make a separate PR that only introduced the checking/verification code from this? That would be a smaller, much less risky change; then this one could be rebased past that, with the higher-risk code isolated and more easily reviewable.
Yes, you are right. I also noticed that I actually was working on two different tasks. But unfortunately both share the same code and also the same tests. So I need to rebuild a separate branch from the master branch and reimport the sub routines from the current branch, and review it of course. Also the second branch would need to be rebased according to which branch is merged first.
I built a new branch at:
Clear Unknown Indices
which implements the --delete --all option and also introduces important tests which are used for the mappings verification tests.
I would really like to see the verify done (including merged and released, since it is inherently less risky) first, not the clear. Is there a good reason why it can't be done in that order?
Actually the verify mappings logic is the bigger part of this development with the introducing the _compare_mapping(), _build_mapping() and _build_aliases() methods with more than 200 lines of code additionally to the tests with another 100 lines of code.
The tests of the verify mappings logic require tests of --create and --update index functionality which require some additional 100 lines of changes.
So my idea was to split the code more evenly and already introduce tests in the smaller task that will thin out the verify mappings development which supposedly would increase the maintainability and reviewabilty of the development.
On the other side the tests of the --delete --all logic show that this functionality is save and should not be able to run in production if my assumptions about the environment recognition are as documented in the related issue:
recognize development environment
flag development environment with environment variables
Still there is also a manual protection as requested at:
manual confirmation required
I rebuilt the "Verify Mappings" functionality at: Verify Indices Mappings which is the semantical split of this development.