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

Bump spec version to 1.0.0-rc.2 #367

Merged
merged 10 commits into from Jun 25, 2020
Merged

Bump spec version to 1.0.0-rc.2 #367

merged 10 commits into from Jun 25, 2020

Conversation

ml-evs
Copy link
Member

@ml-evs ml-evs commented Jun 25, 2020

Closes #329. We need to do this before contributing the openapi schemas to the spec repo. I think we're safe to jump the gun to rc2, as we are the only thing currently blocking it...

@ml-evs ml-evs added the priority/high Issue or PR with a consensus of high priority label Jun 25, 2020
@ml-evs ml-evs requested a review from CasperWA June 25, 2020 12:55
@ml-evs
Copy link
Member Author

ml-evs commented Jun 25, 2020

Hmmm, looks like our regexp for version numbers and the update task doesn't allow release tags like -rc2. I think it's useful to have the tag in the version, but I guess we can just change it to 1.0.0 now. Thoughts?

@ml-evs ml-evs force-pushed the ml-evs/update_spec_version branch from 7e5feae to 25b121a Compare June 25, 2020 13:45
@CasperWA
Copy link
Member

Yeah (and it's -rc.2 as well). Anyway, I think we should just make it v1.0.0? Also, we need to update the tests to be more flexible - or maybe not?

@CasperWA
Copy link
Member

I've taken the opportunity to clean up some stuff.

It seems taking the package version from the installed version is rubbish (sorry @shyamd).
It just kind of screws us over for all these invoke tasks and doesn't properly update the version throughout the code base (where ever __version__ is imported and used), since once would first have to pip install the package to update __version__, which does not seem like an easy thing to do in this workflow.
Instead, I've reverted to the "old" way of simply updating the __version__ in optimade/__init__.py "manually" via the invoke task setver.

Furthermore, the OpenAPI JSON specifications are now automagically updated whenever the package version or support OPTIMADE API spec version is also updated, since this is a step we always have to do to after updating those versions.

Finally, I've updated the package version to the latest actually published version on PyPI: v0.9.1.
The non-updated version was a combined quirk of using the installed package version for __version__ and the "failed" publish Actions workflow.

Finally, the supported OPTIMADE API specification can now support -rc.2 additions. (also a or b instead of rc).
The various places where it's updated throughout the code base has also gotten a cleanup.
This removes the pre-commit for updating the OPTIMADE shield/badge as well, since this is now done whenever the OPTIMADE API spec version is updated.

@shyamd
Copy link
Contributor

shyamd commented Jun 25, 2020

I would still recommend one place of truth for the version. It's one of those things that won't hurt us, but has a high chance of causing a bug for any package that depends on optimade-python-tools. Rather then hardcore in setup.py and __init__.py, can we have a VERSION.txt that gets read in both of these?

@CasperWA
Copy link
Member

I would still recommend one place of truth for the version. It's one of those things that won't hurt us, but has a high chance of causing a bug for any package that depends on optimade-python-tools. Rather then hardcore in setup.py and __init__.py, can we have a VERSION.txt that gets read in both of these?

We can also just have it in the __init__.py file and read it in for setup.py? That's what I normally do. But another file is also fine I guess?

@shyamd
Copy link
Contributor

shyamd commented Jun 25, 2020

That should be fine as long as you don't read it via normal python import.

@CasperWA
Copy link
Member

That should be fine as long as you don't read it via normal python import.

Naturally. All out raw file reading on that one :)

setup.py Outdated Show resolved Hide resolved
@ml-evs ml-evs changed the title Bump spec version to 1.0.0-rc2 Bump spec version to 1.0.0-rc.2 Jun 25, 2020
@ml-evs
Copy link
Member Author

ml-evs commented Jun 25, 2020

Hmmm, looks like our regexp for version numbers and the update task doesn't allow release tags like -rc2. I think it's useful to have the tag in the version, but I guess we can just change it to 1.0.0 now. Thoughts?

This still fails, we need to update our model for available_api_versions, and consider how this messes up versioned urls... I would have thought making this 1.0 is the most sensible solution

@CasperWA
Copy link
Member

Hmmm, looks like our regexp for version numbers and the update task doesn't allow release tags like -rc2. I think it's useful to have the tag in the version, but I guess we can just change it to 1.0.0 now. Thoughts?

This still fails, we need to update our model for available_api_versions, and consider how this messes up versioned urls... I would have thought making this 1.0 is the most sensible solution

Ah, I didn't look at this yet actually. Will fix it asap.

@shyamd
Copy link
Contributor

shyamd commented Jun 25, 2020

There's nothing wrong with adding a tag parameter in the invoke task. That might be even more useful since it's documenting what that means.

@ml-evs
Copy link
Member Author

ml-evs commented Jun 25, 2020

Hmmm, looks like our regexp for version numbers and the update task doesn't allow release tags like -rc2. I think it's useful to have the tag in the version, but I guess we can just change it to 1.0.0 now. Thoughts?

This still fails, we need to update our model for available_api_versions, and consider how this messes up versioned urls... I would have thought making this 1.0 is the most sensible solution

Ah, I didn't look at this yet actually. Will fix it asap.

Looking into it, it's not actually the available_api_versions model (which is just a str) but somewhere else where we check for semver too aggressively.

Nope, its the validator in the model. There's also part of the versioned base url we need to fix too though

@codecov
Copy link

codecov bot commented Jun 25, 2020

Codecov Report

Merging #367 into master will decrease coverage by 0.15%.
The diff coverage is 82.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #367      +/-   ##
==========================================
- Coverage   90.43%   90.27%   -0.16%     
==========================================
  Files          54       54              
  Lines        2352     2386      +34     
==========================================
+ Hits         2127     2154      +27     
- Misses        225      232       +7     
Flag Coverage Δ
#unittests 90.27% <82.50%> (-0.16%) ⬇️
Impacted Files Coverage Δ
optimade/server/routers/index_info.py 100.00% <ø> (ø)
optimade/server/routers/info.py 96.15% <ø> (ø)
optimade/server/routers/utils.py 91.03% <ø> (ø)
optimade/models/utils.py 81.57% <78.78%> (-18.43%) ⬇️
optimade/models/baseinfo.py 94.87% <100.00%> (+0.13%) ⬆️
optimade/models/optimade_json.py 93.90% <100.00%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad68951...b14f3ff. Read the comment docs.

Copy link
Member

@CasperWA CasperWA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found this :)

optimade/server/routers/info.py Outdated Show resolved Hide resolved
optimade/models/utils.py Outdated Show resolved Hide resolved
@ml-evs ml-evs force-pushed the ml-evs/update_spec_version branch from cb4ebe2 to 3e3b4c9 Compare June 25, 2020 18:25
ml-evs and others added 9 commits June 25, 2020 19:48
Package version is made up to speed with the current latest release on
PyPI: v0.9.1
The OPTIMADE API specification is updated to the latest release
candidate version: v1.0.0-rc.2

The invoke tasks have been updated, so that nothing is raised, unless a
true error occurs.
If the package or OPTIMADE spec version is updated, the OpenAPI
specifications are also afterwards.

Instead of getting the package version from the installed version, it is
set through invoke tasks.
This means the `optimade/__init__.py` must be "manually" updated through
the invoke task "setver".
@ml-evs ml-evs force-pushed the ml-evs/update_spec_version branch from cabf187 to 993ea7d Compare June 25, 2020 18:48
CasperWA
CasperWA previously approved these changes Jun 25, 2020
@ml-evs ml-evs merged commit 271a34d into master Jun 25, 2020
@ml-evs
Copy link
Member Author

ml-evs commented Jun 25, 2020

😌

@ml-evs ml-evs deleted the ml-evs/update_spec_version branch June 25, 2020 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/high Issue or PR with a consensus of high priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to v1.0.0-rc.1
3 participants