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

Round versions for upgrade and schema #1038

Merged
merged 4 commits into from Feb 10, 2022
Merged

Round versions for upgrade and schema #1038

merged 4 commits into from Feb 10, 2022

Conversation

danlester
Copy link
Contributor

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 of 0.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 apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features to not work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

Requires testing

  • Yes
  • No

In case you checked yes, did you write tests?

  • Yes
  • No

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.

@costrouc
Copy link
Member

costrouc commented Feb 9, 2022

@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.

Copy link
Member

@costrouc costrouc left a 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.

@costrouc
Copy link
Member

costrouc commented Feb 9, 2022

@tonyfast you might be able to help comment on this and how we could easily be setting the version in development.

@danlester
Copy link
Contributor Author

That could be better really, although we should still keep the bits that remove dependency on packaging. Will see what anyone else says.

@danlester
Copy link
Contributor Author

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 qhub upgrade between 0.4.0 and 0.4.1 (regardless of whether the qhub-config.yaml schema changes) to ensure we pick up 0.4.1 tagged Docker images and Terraform infrastructure. But we don't need to be so strict between dev versions of the same 0.4.0 pre release.

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 qhub_version: 0.4.0 then we can loosen the check within schema.py and let them do it. As a default, I have maintained the check above that a patch change forces this co-ordination via qhub upgrade, but smaller changed (dev/prerelease) do not.

@danlester danlester added this to the Release v0.4.0 milestone Feb 10, 2022
@costrouc costrouc merged commit 97d6edc into main Feb 10, 2022
@costrouc costrouc deleted the roundedversions branch February 10, 2022 20:28
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 this pull request may close these issues.

qhub upgrade misses step if version is a dev release [bug]
2 participants