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
3 stars 4 forks source link

Break installation defaults out as their own structure. #83

Open albu-diku opened 1 month ago

albu-diku commented 1 month ago

Add tests that assert the consistency of options accepted by the generateconfs command line against the internal library function. Additionally assert that the library routine itself matches the defaults structure thus making the structure the definitive source of truth.

Doing so highlighted the following missing command line options which are added as of this commit: --seafile_secret --seafile_ccnetid

jonasbardino commented 1 month ago

The restructure looks good and I was just about to merge when I noticed the py2 unit tests failing as described above. Should we move the _make_parameters helper to a generate_confs_params_spec in mig.shared.install and grab it from there for now?

albu-diku commented 1 month ago

The restructure looks good and I was just about to merge when I noticed the py2 unit tests failing as described above. Should we move the _make_parameters helper to a generate_confs_params_spec in mig.shared.install and grab it from there for now?

So I considered that but actually I'd lean towards not yet - I think the "how we might wan this to be" story is still evolving and it's less churn to leave them there and follow-up once we have clearer picture (some extra context here: https://github.com/ucphhpc/migrid-sync/pull/83#discussion_r1687437149).

jonasbardino commented 1 month ago

Not sure if this overlaps with your recent answer, but while experimenting a bit with that move into mig.shared.install I hit another snag with the py2 unit tests... Event after make distclean I hit an ImportError: cannot import name SimpleNamespace. AFAICT it was not added in the python stdlib until 3.3 so it probably won't fly as such. Perhaps we could use the following workaround if you really, really want to take the class-way: https://opendev.org/x/ospurge/commit/c2d1728deedf7e2ca03e6a55f4de59d4d3ab1dde but otherwise we'd usually just go with a good old plain dict, which seems to be all you really need here.

albu-diku commented 1 month ago

Not sure if this overlaps with your recent answer, but while experimenting a bit with that move into mig.shared.install I hit another snag with the py2 unit tests... Event after make distclean I hit an ImportError: cannot import name SimpleNamespace. AFAICT it was not added in the python stdlib until 3.3 so it probably won't fly as such. Perhaps we could use the following workaround if you really, really want to take the class-way: https://opendev.org/x/ospurge/commit/c2d1728deedf7e2ca03e6a55f4de59d4d3ab1dde but otherwise we'd usually just go with a good old plain dict, which seems to be all you really need here.

Hah yep. The big reason I leaned towards SimpleNamespace as opposed to dict is such that the defaults could be consumed as attributes, but there is so little to SimpleNamespace that I just the handful lines necessary to compat.