Allow Breaking Configuration Inheritance #326 #339
Conversation
To break inheritance, the merge_configs needs to know what to not include. The two recursive functions will search and replace the sections where inheritance is broken.
The merge_configs function was previously not checking if the break-inheritance flag is set at the proper level. The validation function validates if the break-inheritance flag is set at the top-level setting of the provided configuration. The validation function will quit the program if users set the break-inheritance flag improperly. To identify the setting levels more_general_config and more_specific_config refer to, pass keyworded arguments to merge_configs.
Codecov Report
@@ Coverage Diff @@
## main #339 +/- ##
==========================================
- Coverage 80.83% 79.81% -1.03%
==========================================
Files 63 63
Lines 2385 2432 +47
==========================================
+ Hits 1928 1941 +13
- Misses 457 491 +34
|
|
@gdubicki Please let us know if you have another implementation in mind. |
Since the validation function is seperated out, the call to the validation can be done in the wrapper classes, which reduces the complexity of the merge configs function.
The functionality of the merge configs function was out of scope, which made calling the function unclear and the implementation details unclear. By separating the validation function out, it clarified each functions purpose and scope.
|
I did 10 dry runs each: with my changes and without my changes. The times are not that different. It was 4.85 seconds with my changes and 4.80 seconds without my changes. If you have any thoughts or ideas regarding the implementation that would be great. |
There was a problem hiding this comment.
Thank you for the PR, @ep-linden! Sorry if the review is premature as the PR is still in Draft. I wanted to give you some feedback though.
The great thing is that your tests show that it works! 🥳
The slightly worse is that I only scraped over the surface and have pretty trivial comments for you now. Unfortunately I don't understand your core code (yet) so I don't have comments about the logic. :(
Do you plan to change it?
tests/unit/configuration/test_inheritance_break_projects_and_groups.py
Outdated
Show resolved
Hide resolved
tests/unit/configuration/test_inheritance_break_projects_and_groups.py
Outdated
Show resolved
Hide resolved
|
@gdubicki I think the logic will more or less be the same. I will provide a high-level overview of my logic to hopefully help you easily understand the code. The The Hopefully, this made it clearer. |
|
If you want to discuss this more synchronously over chat @ep-linden @amimas @jccl, then please use https://join.slack.com/t/gitlabform/shared_invite/zt-164l3iwxd-hLqZX7v0AU4RsvVajYFtYQ link to join gitlabform's Slack. :) |
With the hierarchical inheritance structure in GitLabForm V2, users cannot break the inheritance. To see a better example of this use case please see Issue #326.