Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Publishing packages.yaml together with packages does not work when using multiple branches. #107

Closed
hugobuddel opened this issue Jun 14, 2023 · 6 comments · Fixed by AstarVienna/ScopeSim#234

Comments

@hugobuddel
Copy link
Collaborator

Today there was an issue (#105) that it was not possible to download the Paranal package.

That was because https://scopesim.univie.ac.at/InstPkgSvr/packages.yaml listed Paranal.2022-04-09 as latest (and stable) version, but that package was not available in https://scopesim.univie.ac.at/InstPkgSvr/locations/ .

Paranal.2022-04-09.zip was probably never uploaded. This might not have been problematic, if indeed packages.yaml would have pointed to the correct file, which might have been the case for a while. However, I 'recently' (March) uploaded a new MICADO package. That publishing would also have uploaded packages.yaml, thereby breaking the Paranal download (for 3 months without anyone telling us). Not sure this was indeed the case though, as older version of packages.yaml are not kept on the server.

However, a similar problem is occurring with MAAT/OSIRIS. packages.yaml in the dev_maat branch contains

irdb/irdb/packages.yaml

Lines 45 to 49 in 23f7fba

OSIRIS:
latest: OSIRIS.2023-01-13
path: instruments
stable: OSIRIS.2023-01-13
Paranal:

while packages.yaml in the master branch (that I uploaded when uploading MICADO) contains

irdb/irdb/packages.yaml

Lines 45 to 49 in 143264e

OSIRIS:
latest: OSIRIS
path: instruments
stable: OSIRIS
Paranal:

Only OSIRIS.2023-01-13.zip exists on https://scopesim.univie.ac.at/InstPkgSvr/instruments/ , but I overwrote the packages.yaml from the dev_maat branch, so now it is not possible anymore to download OSIRIS.

GTC and LaPalma are also broken, see AstarVienna/ScopeSim#225 .

I'm too wary right now to think about the proper solution to this problem. Some ideas, in order of increasing usefulness and difficulty:

  • Easy: Only publish packages from the master branch (or perhaps the dev_master branch for development packages). This would only require changes in our workflow, not in the code.
  • Medium: Get rid of packages.yaml. The filenames (e.g. test_package.2022-04-27.dev.zip contain all the relevant information already, so no need to consult packages.yaml at all. This requires code change though.
  • Hard: Create PyPI packages of all the packages in the IRDB. Then we get this versioning stuff for free. The packages are all rather small in size anyway. I like this the most, but it is the most work.

@teutoburg what do you think?

@hugobuddel
Copy link
Collaborator Author

To clarify, it was trivial to resolve the Paranal problem (#105) simply by uploading it again from the master branch, because the Paranal package is the same in all branches. However, solving OSIRIS is harder. The four options are:

  1. Upload OSIRIS from the master branch. This would lead to an outdated version on the server.
  2. Upload OSIRIS from the dev_maat branch. This would break all other packages like MICADO, because the packages.yaml from the dev_maat branch would be used.
  3. First merge dev_maat into (dev_master into) master, before doing 1 above. This would probably cause problems too, because if this merging would have been possible, it would probably have been done when OSIRIS.2023-01-13 was released.
  4. Merging master into dev_maat, before doing 2 above. This would then have to be done every time any package is released, and cannot be done if there are more such dev_instrument branches.

@teutoburg
Copy link
Contributor

I think the PyPi option might be worth considering in the long run, but probably too much effort for now...
Let's consider your medium option next:
The only extra information that packages.yaml seems to provide is the distinction between a latest and a stable version of each package. These are identical except for the test_package. I don't know what the original intention was for having this distinction, but I'm wondering if that couldn't be done in a better way, like adding .dev as it's done for the test_package.

What I still don't fully understand is why every package version is updated in packages.yaml, whenever we update any package. There's most likely a good reason for this, that I'm missing, but I would have expected that only the version of the package(s) that actually got updated would need to change.

@teutoburg
Copy link
Contributor

Regarding the OSIRIS and dev_maat issue, it seems if we go for the medium option, then the OSIRIS option no. 2. would work? Or indeed, if packages.yaml would not update every package version number.
If we do upload OSIRIS from dev_maat, it should probably be considered a latest version ending on .dev, while the stable version is the (outdated) one from the master branch.

@teutoburg
Copy link
Contributor

While we talk about all of these version issues, what do you think about using sematic versioning for IRDB instead of the date?

@hugobuddel
Copy link
Collaborator Author

There's most likely a good reason for this, that I'm missing,

Maybe; that's Chesterton's Fence. But I don't think it applies here, it seems to have grown the other way around. First there was only packages.yaml and then later also the individual version.yamls and such.

I think we can indeed just do with the filenames and remove relying on packages.yaml. Perhaps we should hardcode the paths, as those are only a few, and then just look at what zipfiles these directories contain. The latest should then indeed end with .dev, and stable not. However, if there is a stable with a later date than a .dev, then that should also count as the latest.

I think dates (a bit like CalVer) is fine for this purpose. As changes to the IRDB should also reflect changes to the hardware. I don't care much, but I don't think it is worth to change the versioning scheme. I've added tags for HAWKI-2023.06.14 and Paranal.2022.04.09 to the dev_master branch, even though technically the Paranal one was from #106 .

A bigger problem is that apparently we uploaded broken packages, see the HAWKI comment on #105. We should move towards an automatic scheme that prevents broken releases. A CalVer like scheme makes automation easier, because you don't have to have a human in the loop to decide whether the to-be-made release is a major, minor, or patch release.

@teutoburg
Copy link
Contributor

Considering these things, I do agree about the versioning scheme. But it's good to remind ourselves why we do things the way we do them :)

If I would have created the code to download these things from the server, I probably would have either gone with hardcoded paths as well, or with (the url version of) glob. But the distinction in locations, telescopes, instruments makes sense IMO, adding some meaningful structure. If we want, we might include a way to "list locations", "list telescopes" or "list instruments" in the future, just as conveniences for the enduser. Or e.g. "list all available instruments for this telescope" or "list all available telescopes for this location", or a tree-like combination. Just a few ideas that popped into my head :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants