QC -- do not merge yet

[o-smirnov]

No description provided.

[o-smirnov]

@landmanbester @JSKenyon could you please take a look at this branch? Would be nice to straighten it out and merge.

[JSKenyon]

Shouldn't this just the latest PyPI release?

[o-smirnov]

I believe so, now that 0.2.2 is out.

[JSKenyon]

The one thing I am a little unclear on is how we handle changes in the QC config. For each QC version, we technically have to copy across all the schemas, and other supporting files. The correct version will need to be used based on the version of QC the user selects. Doing this by hand might get very tedious - is there an alternative?

[o-smirnov]

Doing this by hand might get very tedious - is there an alternative?

I don't have a fully-formed solution in mind yet. Does the schema change every version? I've been kicking this can down the road for now. My feeling is we can get one release out without worrying too much about it, and have something in place for the next release.

[JSKenyon]

Doing this by hand might get very tedious - is there an alternative?

I don't have a fully-formed solution in mind yet. Does the schema change every version? I've been kicking this can down the road for now. My feeling is we can get one release out without worrying too much about it, and have something in place for the next release.

No, it shouldn't change on every release (only if/when I change/add options). That is probably fine for now. The cadence of updates should be irrelevant though. I think that the correct thing to do is to have a github action that handles this automatically. This could be on either the application repo or on cult-cargo. The latter would be preferable but the former is probably easier. Basically, on release, the action should pick out the required files (a reason to have this on the application repos) and automatically make a PR into cult-cargo pushing those files to some folder based on the current release version. This means that the application developer only has to worry about it once, and it simply becomes merging a PR on the cult-cargo end.

[JSKenyon]

@landmanbester I think that I have taken care of the merge conflicts and QC related stuff here. I have not tested it myself yes as I presume that @o-smirnov has run QC using these changes at some point. I am not sure about the pfb-clean stuff though e.g. the commented out dependency etc.

[landmanbester]

Shall we divorce this PR from all the pfb stuff and just get qc in? I can then merge this into my branch and get all the pfb stuff working

[landmanbester]

Oh, I see all the configs are in there already. Taking a look now

[landmanbester]

Some dependency woes I need to attend to before this can go in. Will try get it done asap

[landmanbester]

After some setuptools woes I managed to install pfb-clean in python3.9 but now I am getting

Successfully tagged quay.io/stimela2/pfb-clean:cc0.1.2
0:35:23 ⠏ image pfb-clean [0/1]: version latest [0/1]
Traceback (most recent call last):
  File "/home/landman/software/cult-cargo/./cultcargo/builder/build-cargo.py", line 361, in <module>
    build_cargo()
  File "/home/landman/venvs/ccargo/lib/python3.10/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File "/home/landman/venvs/ccargo/lib/python3.10/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File "/home/landman/venvs/ccargo/lib/python3.10/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/landman/venvs/ccargo/lib/python3.10/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File "/home/landman/software/cult-cargo/./cultcargo/builder/build-cargo.py", line 299, in build_cargo
    if image_version == tag_latest[image]:
KeyError: 'pfb-clean'

even though this is clearly in the manifest

cult-cargo/cultcargo/builder/cargo-manifest.yml

Line 153 in ad873f4

pfb-clean:

I'm finding build-cargo.py very annoying to debug because of the progress bar. Is there a way to disable it and shouldn't this be the default?

[o-smirnov]

Is there a way to disable it

Yes, disable=true 😎

Try again, I added a --boring option just for you.

and shouldn't this be the default?

No.

[landmanbester]

I've pushed an updated image, everything should be working now. I've added a (bit of a kludgy) script to pull the pfb-clean config from the pfb-clean repo. Eventually we should probably do this in a better way but at least makes the process less error prone for the time being

[o-smirnov]

@landmanbester @JSKenyon, I left some minor comments.

[o-smirnov]

@JSKenyon should this not be a URI then?

[JSKenyon]

I have updated the backup and restore cabs - could you please give them a once over to check that I haven't done something unholy @o-smirnov?

[o-smirnov]

@JSKenyon same here.

[JSKenyon]

I have updated the backup and restore cabs - could you please give them a once over to check that I haven't done something unholy @o-smirnov?

[o-smirnov]

LGTM, apart from that minor typo.

[o-smirnov]

@landmanbester @JSKenyon one of you needs to leave an approving review, then we can merge.

[JSKenyon]

This all looks good to me.

[landmanbester]

Just a sec