search

ucphhpc / migrid-sync

MiGrid workspace where master branch is kept strictly in sync with SF upstream svn repo. Any development or experiments should use a branch. You probably want to fork your own clone or work e.g. on the edge branch if you wish to contribute.
GNU General Public License v2.0
4 stars 4 forks source link

Add a command line option for permanent_freeze to cnfig generation. #104

Closed albu-diku closed 3 months ago

jonasbardino commented 3 months ago

Manually merged through svn.

jonasbardino commented 3 months ago

Hmm, so far so good but testing some more it turns out there's one important part missing - namely support for the space-separated list of archive types to permanently freeze, which is what we need in production: --permanent_freeze="freeze backup" That is, it's actually not a boolean option but just a plain string passed as-is and handled in the configuration - I should have explained that of course :-s Anyway, it's not too complicated to address now, so I'll just open a new PR doing that.

albu-diku commented 3 months ago

Hmm, so far so good but testing some more it turns out there's one important part missing - namely support for the space-separated list of archive types to permanently freeze, which is what we need in production: --permanent_freeze="freeze backup" That is, it's actually not a boolean option but just a plain string passed as-is and handled in the configuration - I should have explained that of course :-s Anyway, it's not too complicated to address now, so I'll just open a new PR doing that.

So, this was one of the cases that I'd intended to be handled - I think what it highlights is that I missed adding a test for that case. I know how this happened, I added the test cases for the actual argument processor late in the game and it slipped by me to explicitly test this. As a side note it also highlights the handling of that argument is a little messy and does not "just work" the way it ought to, but dealing with that is for another day. At any rate apols for missing this.

jonasbardino commented 3 months ago

Yes, it's very easy to start something and then forget about it while juggling other issues. I know because I did just that thinking about testing the space-separated string case before merging, but then forgot about it again and ended up needing the a follow-up PR109 :see_no_evil: :sweat_smile:

Agreed that the generator and the argument handling is not in the greatest shape after massive 'organic' growth. A rethinking/reworking of it is welcome.

albu-diku commented 3 months ago

Agreed that the generator and the argument handling is not in the greatest shape after massive 'organic' growth. A rethinking/reworking of it is welcome.

The direction I thought to head on that front is roughly here in draft form: https://github.com/ucphhpc/migrid-sync/pull/99