-
Notifications
You must be signed in to change notification settings - Fork 97
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
Round versions for upgrade and schema #1038
Conversation
196d4e9
to
9176877
Compare
@danlester there is a hook https://github.com/pypa/setuptools_scm#environment-variables for setting the version via environment variables for development. Have you tried this? I'd prefer to keep the version logic simple. I do understand that this makes it so you have to set We don't need to worry about this version issue when we deploy to pypi and conda since the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danlester there is a hook https://github.com/pypa/setuptools_scm#environment-variables for setting the version via environment variables for development. Have you tried this? I'd prefer to keep the version logic simple. I do understand that this makes it so you have to set SETUPTOOLS_SCM_PRETEND_VERSION=0.4.0
for example before running the pip install .
.
We don't need to worry about this version issue when we deploy to pypi and conda since the _version.py
file will be created.
@tonyfast you might be able to help comment on this and how we could easily be setting the version in development. |
That could be better really, although we should still keep the bits that remove dependency on |
SETUPTOOLS_SCM_PRETEND_VERSION isn't really going to work, e.g. for upgrade pytests, because it needs to be set before pip install, and I think that's overkill just for those tests. We want to test as much as possible with the 'real' package I think. I have updated the PR to rely less on rounded versions, but these are still important for comparisons to decide which upgrade steps should be included in an upgrade. We talk about the schema version of qhub-config.yaml, but really this should be considered in conjunction with infrastructure and docker image versioning. It's possible that the qhub-config.yaml schema will remain the same between multiple releases of qhub, but we still don't want users to redeploy blindly with a new qhub release because the infrastructure may have changed. So we force them to do a Ultimately, if we are happy with the user deploying e.g. 0.4.1 QHub but with 0.4.0 Docker images and a qhub-config.yaml marked with |
Fixes #1037
Changes:
The version number applying to
qhub-config.yaml
schema and qhub Docker images as specified in the config file is now based on the 'rounded version' rather than the precise version of QHub.This means that we use e.g.
0.4.0
instead of0.4.0.dev65+g2de53174
.Thus a qhub config generated with a dev or prerelease version will still be compatible when upgraded to the full release.
Importantly, this allows the Upgrade functionality to be defined with reference to full release version numbers without having to worry about dev/prereleases etc. As in issue #1037 this complication caused the intended final upgrade step to fail to run because 0.4.0.prerelease123 is 'earlier' than 0.4.0 so it doesn't want to run the step to upgrade to 0.4.0.
This also removes the dependency on the
packaging
Python package.Types of changes
What types of changes does your code introduce?
Put an
x
in the boxes that applyTesting
Requires testing
In case you checked yes, did you write tests?
Further comments (optional)
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered and more.