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

add automatic PyPI deployment #75

Merged
merged 4 commits into from
Nov 11, 2019

Conversation

ltalirz
Copy link
Member

@ltalirz ltalirz commented Nov 7, 2019

This automatically deploys any tags of a valid form v0.1.2 to PyPI using an encrypted PyPI API token (which provides access only to the optimade PyPI package).

I haven't tested this for this specific repo but I'm using the same approach in a number of other packages and I think it should work (one change: I used python 3.7 here to deploy, I guess this should be ok).

In case there is any issue, we can fix it when this is triggered with the next tag.

@ltalirz ltalirz requested a review from ml-evs November 7, 2019 12:31
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

This looks useful, happy to accept pending my query

.travis.yml Outdated
secure: jydagLMQt10k0l/XlVqYznqnfRpXDsnrPIDzxREBMwnoCWBvx7a+X/pEg2wFWMD3uY3/Qhev8dr6+RBgX0t1N0si4TpBMFKGkilMebRbHkkbdgRA19NSQTdgj3jml+3K095i1LO8xR7bSfn/XHZ+oN02EDdhCGEDnCLMoG9AGKLX0w/8lNu4clQxHMQ2hSTeT9imJrD6kZDDTJwMS6twF6QLwE7YowfaPRlqIn6EYjQJjDYLE6Px95v9WBeCdNuL6Yr+ojO8uiQvmj1WdpnxVG1LBYtpx3R5cb2bbj/Q8WAYniXmJGeB0lnOMqtmbDire3kZcn40JfJMJ47bNY79APkQqTylxByv1NomeCf3h4UQ0Ylr3kaKufif+PCixl7yG1YUrnOeE2Rzvtr9YyN9uJ8solpVooOtKnwWQwCkS/pcyi8h2CyfdSBLDlWai7ShkQHRCf2YDqxctyPbqaSar6ezUwT1lKtaFNdPaNfl6Au/PVJMr0ANp3Bfl74EXzoajzc9SlJBbAg/CRTbGq+Mfa8hZUtMwkSJCGcrcqz18x6rlTek/p/fmic8fdVjLSiD4V75q2r6ZrHT7OvmHUxDJ4JA4GwuWFMLNC4813ydXaC+rk3h4MxRnskSVs1v5YcBnb/7DudVWQa6ZAAOesbjqRtj8RfMLWrxW8BvBUUwKyI=
on:
repo: Materials-Consortia/optimade-python-tools
all_branches: true
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to push all branches or just master?

Copy link
Member Author

Choose a reason for hiding this comment

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

from my experience this can be useful for alpha/beta releases that one does not yet want to merge into master, but it comes from the gitflow model where you would have a develop branch for this.
up to you, I can also restrict it to master.

Copy link
Member

Choose a reason for hiding this comment

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

I just worry that everyone has push access to arbitrary branches except master at the moment, (maybe even to master, need to check), so someone could hijack the PyPI version with unreviewed code. (I'd also be fine with moving to the gitflow model!)

@CasperWA
Copy link
Member

CasperWA commented Nov 8, 2019

Great! Thanks @ltalirz.

This automatically deploys any tags of a valid form v0.1.2 to PyPI (...)

I was wondering if it would be possible to have the form v0.1.2.3? Then we could have the first three be major.minor.patch of the OPTiMaDe spec., and the last be the specific revision for this repo.

@ltalirz
Copy link
Member Author

ltalirz commented Nov 8, 2019

I was wondering if it would be possible to have the form v0.1.2.3? Then we could have the first three be major.minor.patch of the OPTiMaDe spec., and the last be the specific revision for this repo.

It's certainly possible but I don't think it is a good idea to couple the version numbers of optimade with the version numbers of this repository.
I would vote to stick to semantic versioning for this package in order to be able to signal bug fixes / new features / breaking changes.

@ltalirz
Copy link
Member Author

ltalirz commented Nov 8, 2019

I certainly agree, though, that it is important to clearly display, which optimade version the python tools are implementing.
It should be possible to get this information programmatically from python, and perhaps we should even display it in the README of the repo?

@CasperWA
Copy link
Member

CasperWA commented Nov 8, 2019

It's certainly possible but I don't think it is a good idea to couple the version numbers of optimade with the version numbers of this repository.

This is exactly what was proposed in #67, but to have only major.minor be equal to (not patch).

I would vote to stick to semantic versioning for this package in order to be able to signal bug fixes / new features / breaking changes.

I don't know. I would like to have both 😅 Whether this be via v(OPTiMaDe spec version).(optimade-python-tools version) or something of those lines? Would that be too much?

I see this repository mainly for the value of the pydantic models, which may be used by any implementation to validate your response.
The server part of this I see merely as an example of how such an implementation can be done.
Maybe it would make more sense then to split the repo up, keeping the models (and soon to come external API validator) in this repo and then a "test" server implementation in another repo?

@ml-evs
Copy link
Member

ml-evs commented Nov 8, 2019

We should also consider how we're going to support multiple versions of the spec at once, e.g. validate implementation against v1.0 or future v2.0. This is worth a separate issue but would definitely effect the version numbers for this repo.

Do we a) manually version the models inside this repo (e.g. models/latest, models/v1.0, models/v1.1, b) split the models into a separate package so switching version is just a case of pip install optimade-python-models==0.9.7 then have that extra package exactly follow the spec version, or c) something better?

I guess the example server code will only ever track the "latest version" of the spec (or should it go in a separate versioned package with the models?), but the validator/grammar should be usable against arbitrary versions...

@fekad
Copy link
Contributor

fekad commented Nov 8, 2019

It's certainly possible but I don't think it is a good idea to couple the version numbers of optimade with the version numbers of this repository.

This is exactly what was proposed in #67, but to have only major.minor be equal to (not patch).

I would vote to stick to semantic versioning for this package in order to be able to signal bug fixes / new features / breaking changes.

I don't know. I would like to have both 😅 Whether this be via v(OPTiMaDe spec version).(optimade-python-tools version) or something of those lines? Would that be too much?

I see this repository mainly for the value of the pydantic models, which may be used by any implementation to validate your response.
The server part of this I see merely as an example of how such an implementation can be done.
Maybe it would make more sense then to split the repo up, keeping the models (and soon to come external API validator) in this repo and then a "test" server implementation in another repo?

Please correct me if I'm wrong but the GitHub tag is completely independent from the version number used by pypi and defined in setup.py. Of course, we can use the same version for both but it has to be modified at the same time...

@ltalirz
Copy link
Member Author

ltalirz commented Nov 10, 2019

To me, the point by @ml-evs above concerning support for multiple API versions is just another reason not to couple the versioning of the package to an API version. From my point of view this PR is ready to be merged.

Please correct me if I'm wrong but the GitHub tag is completely independent from the version number used by pypi and defined in setup.py.

Correct.

Of course, we can use the same version for both but it has to be modified at the same time...

Not quite sure what you are trying to say here...
anyhow, from my point of view there is I see no good reason to use different conventions for tags (which will be shown as "releases" on github) and releases on pypi.

@ml-evs
Copy link
Member

ml-evs commented Nov 11, 2019

Cheers @ltalirz , I guess we can move the versioning discussion to an issue

@ml-evs
Copy link
Member

ml-evs commented Nov 11, 2019

Travis isn't running on this branch for some reason...

@ltalirz
Copy link
Member Author

ltalirz commented Nov 11, 2019

Feel free to merge whenever you see fit.
Note that the extra travis job won't show up here (only in the build triggered by pushing a tag of the right format).

@ml-evs
Copy link
Member

ml-evs commented Nov 11, 2019

Feel free to merge whenever you see fit.
Note that the extra travis job won't show up here (only in the build triggered by pushing a tag of the right format).

The actual PR isn't showing up on travis so it can't be merged (unless we change our settings). Do you have any strange permissions on your fork?

@ml-evs ml-evs merged commit 187c1f8 into Materials-Consortia:master Nov 11, 2019
@ml-evs
Copy link
Member

ml-evs commented Nov 11, 2019

Temporarily changed settings and merged as there was no way to trigger a travis build.

@ml-evs
Copy link
Member

ml-evs commented Nov 11, 2019

It seems that this PR actually breaks normal travis builds. (Unfortunately travis status was reporting some issues when I was merging this, so just assumed it was that...)

I can't even trigger a build with a custom config that excludes the changes in this PR, so really not sure what is going on.

@fekad
Copy link
Contributor

fekad commented Nov 11, 2019 via email

@ltalirz
Copy link
Member Author

ltalirz commented Nov 11, 2019

@fekad I think that's on me here - very sorry about this.
I'll look into what I did wrong here...

@ltalirz
Copy link
Member Author

ltalirz commented Nov 11, 2019

It seems I ran into this slightly weird travis issue: travis-ci/travis-ci#8536

My other packages use builds on multiple python versions/with different environment variables, and there you can keep your tests defined a the top level, with the deploy stage as a job.
When you have only one build (as we have), for some reason you need to specify the tests as a separate job (see build on my fork: https://travis-ci.org/ltalirz/optimade-python-tools/builds/610523570).

So, we have two choices:

@ml-evs Let me know what you prefer

@ml-evs
Copy link
Member

ml-evs commented Nov 11, 2019

Looks like fun... I guess we may as well test python 3.8 too, but I'm not fussed either way!

@ltalirz ltalirz mentioned this pull request Nov 11, 2019
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.

4 participants