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.

Sign up for GitHub

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

Jump to bottom

Finish adding deploy testing with TestPyPI #436

Merged
merged 10 commits into from
Apr 11, 2019
Merged

Finish adding deploy testing with TestPyPI #436

merged 10 commits into from
Apr 11, 2019

Conversation

matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Apr 11, 2019 •
edited
Loading

Description

Completes PR #429 (which was merged early by accident) and resolves #425 and resolves #432.

Inspired by Issue #410 and @webknjaz's tweet which inspired Issue #425 to have more robust deployment to PyPI this PR implements PR level deployments to TestPyPI that are versioned with version structure of {next_version}.dev{distance} through use of setuptools-scm and setuptools-scm-git-archive and heavy copying of ansible-lint's approach.

The level of deployment is set to push commits on master as we commit frequently enough that there isn't a need to pollute the area with a huge number of deployed versions.

As an example of the system working, by turning on deployment from all branches this PR deployed pyhf 0.0.17.dev23 on TestPyPI.

This PR also sets up the path to then complete Issues #410 and #399.

Checklist Before Requesting Reviewer

  • Tests are passing
  • "WIP" removed from the title of the pull request
  • Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

  • Summarize commit messages into a comprehensive review of the PR
- Add deployment of push commits to master to TestPyPI for deploy testing. This uses a TESTPYPI_UPLOAD environment variable to signal to setuptools-scm through setup.py.
- Add setup_requires of setuptools-scm and setuptools-scm-git-archive through setup.cfg
- Add .git_archival.txt and .gitattributes as required for setuptools-scm-git-archive
- For TestPyPI deploy job use type=push to avoid triggering on unintended builds

@matthewfeickert matthewfeickert self-assigned this Apr 11, 2019
@matthewfeickert matthewfeickert added docs Documentation related feat/enhancement New feature or request tests pytest labels Apr 11, 2019
webknjaz
webknjaz reviewed Apr 11, 2019
View reviewed changes
.travis.yml Outdated Show resolved Hide resolved
webknjaz
webknjaz reviewed Apr 11, 2019
View reviewed changes
setup.py
return (
{'local_scheme': lambda version: ''}
if getenv('TESTPYPI_UPLOAD') == 'true'
else False
Copy link
Contributor

Choose a reason for hiding this comment

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

This will cause a fallback to the version hardcoded in python code on line 81.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is the intended behavior. We only want scm versioning for TestPyPI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see :)
It's an interesting approach which I haven't met before...

Copy link
Contributor

Choose a reason for hiding this comment

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

But be aware that this may cause version mismatches between tag and dist because of human errors

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the main reason is that we currently control everything through bumpversion, and I wanted to keep this PR as atomic as possible. We could consider changing things in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't have GitHub actions on pyhf yet! But we are very much looking forward to using it when we get it, so we'll be following up with you! :)

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to sign up first https://github.com/features/actions/signup

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed. I have it for my personal account, but our org doesn't yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, in this case, I invite you to complete this tutorial https://tutorial.octomachinery.dev :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! That looks great. :)

@matthewfeickert matthewfeickert marked this pull request as ready for review April 11, 2019 08:50
webknjaz
webknjaz reviewed Apr 11, 2019
View reviewed changes
setup.cfg
@@ -3,3 +3,8 @@ universal = 1

[metadata]
license_file = LICENSE

[options]
setup_requires =
Copy link
Contributor

Choose a reason for hiding this comment

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

You know that with old setuptools this won't work, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup. But deployments happen with Python 3.7 and modern setuptools. We're in the slow process of starting the migration of getting ready to drop Python 2.7, but our field (high energy physics) is still using older everything so we haven't gone Python 3 and modern libs only yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, as long as you ship binary wheels building them yourself it shouldn't matter.

@coveralls
Copy link

coveralls commented Apr 11, 2019 •
edited
Loading

Coverage Status

Coverage remained the same at 97.482% when pulling dc270a6 on fix/finish-testing-with-TestPyPI into 324b8ca on master.

@matthewfeickert matthewfeickert merged commit 278ac80 into master Apr 11, 2019
@matthewfeickert matthewfeickert deleted the fix/finish-testing-with-TestPyPI branch April 11, 2019 14:44
@matthewfeickert
Copy link
Member Author

@webknjaz Thanks very much for being so generous with your time to give help and feedback! 👍

@webknjaz
Copy link
Contributor

You're welcome :)

@matthewfeickert matthewfeickert mentioned this pull request Jan 5, 2020
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webknjaz webknjaz webknjaz left review comments

@kratsg kratsg kratsg approved these changes

@lukasheinrich lukasheinrich Awaiting requested review from lukasheinrich

Assignees

@matthewfeickert matthewfeickert

Labels
docs Documentation related feat/enhancement New feature or request tests pytest
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test Deploy feature is broken Publish to Test PyPI to ensure robust release pipeline
4 participants
@matthewfeickert @coveralls @webknjaz @kratsg