Skip to content

Use dynamic_versioning as backend #448

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

scarlehoff
Copy link
Member

@scarlehoff scarlehoff commented Mar 10, 2025

put dynamic_versioning as the backend so that one gets 0.14.whatever instead of 0.0.0 when using it locally, not sure whether that will mess up with something?

closes #365

@felixhekhorn felixhekhorn added the output Output format and management label Mar 10, 2025
@felixhekhorn felixhekhorn self-requested a review March 10, 2025 12:50
@felixhekhorn
Copy link
Contributor

I also put dynamic_versioning as the backend so that one gets 0.14.whatever instead of 0.0.0 when using it locally, not sure whether that will mess up with something?

Mmm, that is dangerous: because e.g. master is at the moment definitely NOT 0.14 ... and as I was arguing in #417 with @evagroenendijk it is wrong to call master v0.15, because v0.15 might (or might not) contain even more breaking changes than now; so I think I prefer v0.0 - which should never appear in public codes any ways, but only for developers (who can deal with it)

@scarlehoff
Copy link
Member Author

scarlehoff commented Mar 10, 2025

which should never appear in public codes any ways, but only for developers

But in practice we have many fktables and ekos produced with 0.0.0.

But I do see the point, perhaps a better solution would be to (for a developing installation) get the commit without the tag?

Edit: perhaps I didn't explain myself well in the OP, 0.14.whatever is 0.14.6+<number of commits since tag>+<commit hash>

@felixhekhorn
Copy link
Contributor

But I do see the point, perhaps a better solution would be to (for a developing installation) get the commit without the tag?

is it possible to just have the commit?

0.14.6+<number of commits since tag>+<commit hash>

or do we always need the full thing?

in any case we will need to adjust the legacy handling (initialised in #417) - specifically, we need to adjust this logic:

version = parse(raw["version"])

and upload a new file version as explained here https://github.com/NNPDF/eko/blob/master/tests/data/assets.sh

@felixhekhorn
Copy link
Contributor

At the moment if EKO reads an object through a symlink and it is not in readonly mode, it will overwrite the symlink instead of the original file.

This PR modifies read so that it goes to the original file instead.

do you want to split this change from this PR? since I guess that change is uncontroversial (and does hopefully have no tail)

@scarlehoff
Copy link
Member Author

scarlehoff commented Mar 10, 2025

is it possible to just have the commit?

Yes. But

in any case we will need to adjust the legacy handling (initialised in #417) - specifically, we need to adjust this logic:

How does the legacy handler deals with 0.0.0 since it can be any version? (and adding the commit won't solve that)

(edit: in any case, I added the versioning because I thought it would have no downstream effects, since it does let's leave it be)

@felixhekhorn
Copy link
Contributor

How does the legacy handler deals with 0.0.0 since it can be any version? (and adding the commit won't solve that)

ehm simply this

if version.major == 0 and version.minor == 13:

and this
elif version.major == 0 and version.minor == 14:

is false and so no recover is applied - i.e. it assumes it is master. We only support versions after v0.13.5

  • if we do 0.14.whatever we have to change the conditions: i.e. check for data_version first
  • if we do something else, we need to check if packaging likes that and then adjust the conditions if necessary

@felixhekhorn felixhekhorn changed the title Don't overwrite symlinks, overwrite actual file instead Use dynamic_versioning as backend Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
output Output format and management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version being saved as 0.0.0
2 participants