Feature/pep517 release #226

jaraco · 2019-09-26T19:31:34Z

❓ What kind of change does this PR introduce?

❓ What is the current behavior? (You can also link to an open issue here)

The release process is embedded in a Travis config, so it's difficult to run manually.

❓ What is the new behavior (if this is a feature change)?

In addition to providing for a manual release process (for when the automated process is failing, such as it is now), this technique leverages pep517 to build the package, supporting isolated builds and best practices for packaging.

📋 Other information:

📋 Checklist:

I think the code is well written
I wrote good commit messages
I have squashed related commits together after the changes have been approved
Unit tests for the changes exist
Integration tests for the changes exist (if applicable)
I used the same coding conventions as the rest of the project
The new code doesn't generate linter offenses
Documentation reflects the changes
The PR relates to only one subject with a clear title
and description in grammatically correct, complete sentences

This change is

jaraco · 2019-09-26T20:04:53Z

This change also fixes #214.

webknjaz · 2019-09-26T20:11:31Z

.travis.yml

-        secure: RAfz06AINvz7bfij/YhfkAreRqamgxS8a6jSRNxntYhtJke3ZszUbIDag8+n1I+G5XT2LnMhHqPNR7Plc+AeMz7VXTuy+b81Li5kse20NYlPhd7mBVmTUpXtqYQashV5J39F4qkATBLznCOrMEomM07VTXjO/o2hmQuXniab2Uo=
-      distributions: dists
-      skip_cleanup: true
+    install:


This is not needed, just add tox-venv to tox.ini and set

Suggested change

install:

env:

TOXENV: release

I'm not sure I want to invoke tox -m tox --notest or *python_ssl_openssl_version or python -m OpenSSL.debug during install, which is why I chose to override it.

Also, I'm not sure about adding tox-venv to tox.ini. There may be bootstrapping issues. If tox needs a later version or needs to install tox-venv, then it will create a .tox/.tox env in which to install those packages. If tox-venv isn't already installed (or is), that may affect how .tox/.tox is created. The only technique I trust is to install tox-venv alongside tox prior to invoking .tox. I'm open to experimenting with other techniques, but I don't want to adopt those risks as part of this change.

Okay, so let's change this to installing tox-venv and invoking tox with --notest because --notest allows us to clearly separate envs bootstrap (hence the install step) from actually running main commands (script).
Using env allows us to unify interactions with tox across the config file.

Oh, I see. By running --notest, you get bootstrapping of the environment(s) before running their commands. That's interesting and does make sense. I'm reluctant to add complication, but I can see the benefit there. Let's explore that after validating this simpler approach.

Yep, that was my point :)

Oh, this discussion got distributed over comments... #226 (comment)

webknjaz · 2019-09-26T20:11:39Z

.travis.yml

-      skip_cleanup: true
+    install:
+    - python -m pip install tox tox-venv
+    script:


This won't be necessary too.

Done in 31150f6. I'm slightly reluctant to use an environment variable for a one-off where a command-line switch will do, as the latter provides a nice example for a user as to what to do to release, whereas setting an environment variable for the duration of a command is harder and varies by environments (shell, OS, etc).

webknjaz · 2019-09-26T20:14:43Z

tox.ini

+    TWINE_USERNAME = {env:TWINE_USERNAME:__token__}
+commands =
+    python -c "import path; path.Path('dist').rmtree_p()"
+    python -m pep517.build .


I usually prefer having separate build and publish envs. Then we'd do smth like TOXENV=build-dists,setup-check,release.

Three steps for one artifact? I dunno. That sounds overly complicated. Then you'd have to ensure that the artifacts from each invocation are shared across the three steps... and a manual release would then require the same steps (or at least the build/release steps). I can imagine a few scenarios where it might be useful to have separate steps, but 99% of the use-cases are going to be to invoke the suite of commands, so I'd prefer to keep it simple. And in that 1% of the cases where it's necessary to separate the concerns, one could use tox --notest and run the commands incrementally. Why not just keep it simple and concise?

I think it does too much. I often want a command that gives me dists what I can then inspect with other tools. How about something like this https://github.com/pypa/twine/blob/5cb1867/tox.ini#L53-L61 then?

Also, ideally in the future, I'd like to try out a better testing strategy: first build the artifacts, then pip-install from there and completely avoid lib imports from the repo.

I'd like to try out a better testing strategy

Yes, maybe. I'm heavily invested in the test-at-source methodology because it's faster. It's one of the reasons I prefer Python to (your language here). I do acknowledge there's weakness in testing against the source instead of the built artifact... but my instinct there would be to provide additional testing for the build artifact rather than encumber the development process with two sources of truth (source and installed code).

Yeah, I think that for the development purposes it's more comfortable to rely on editable installs. And yes, I do agree that we need more artifact testing in CI than locally. I've been meaning to implement this for a while.
Here's a nice article that Paul published this topic last month: https://blog.ganssle.io/articles/2019/08/test-as-installed.html

webknjaz · 2019-09-26T20:17:58Z

Now that I released https://github.com/marketplace/actions/pypi-publish, I'd want to start using it. But this would not wait on Travis CI out of the box (yet doable) so I'd probably set a "manual" trigger which would be either a "Release" button on the Checks page (I have a working app for that) or publishing a release @ https://github.com/cherrypy/cheroot/releases/new.

codecov · 2019-09-26T20:24:50Z

Codecov Report

Merging #226 into master will increase coverage by 0.46%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #226      +/-   ##
==========================================
+ Coverage   74.51%   74.98%   +0.46%     
==========================================
  Files          23       23              
  Lines        3634     3634              
==========================================
+ Hits         2708     2725      +17     
+ Misses        926      909      -17

webknjaz · 2019-09-26T20:27:14Z

.travis.yml

-      distributions: dists
-      skip_cleanup: true
+    install:
+    - python -m pip install tox tox-venv


Suggested change

- python -m pip install tox tox-venv

- python -m pip install tox tox-venv

- python -m tox --notest

Thanks but no thanks. I'm not going to add a command and complication when the default process does the same thing. Better to rely on tox to do what it does best, rather than try to micro-manage it. This change seems counter to best-practices... and I can hardly imagine the benefit.

I consider it best practice. Seen it used in a lot of places. tox itself uses --notest as it was designed for such separation: https://github.com/tox-dev/azure-pipelines-template/blob/master/run-tox-env.yml#L118

jaraco · 2019-09-26T20:39:51Z

Now that I released https://github.com/marketplace/actions/pypi-publish, I'd want to start using it. But this would not wait on Travis CI out of the box (yet doable) so I'd probably set a "manual" trigger which would be either a "Release" button on the Checks page (I have a working app for that) or publishing a release @ https://github.com/cherrypy/cheroot/releases/new.

All fine and good. I'm happy to adopt any low-friction workflow you'd like to implement... but it needs to work and have low-friction fallbacks when the automated process fails. Some features of the release flows in the projects I support:

web browser not required to cut a release (just tag and push)
command-line not required to cut a release (just tag in a browser)
one-step release process (just tag)
easy fallback when automation fails (force deploy in CI if blocked by [rare] flaky tests or run tox -e release if that doesn't work).

I want this project to have similar characteristics.

jaraco · 2019-09-26T20:43:07Z

I'm testing this technique out now with the 7.0 release.

webknjaz · 2019-09-26T20:43:16Z

Yeah, that could be done. GitHub has a deployment event which can be produced via API or a browser app with a button, and workflows can be triggered with that event too.

jaraco · 2019-09-26T21:00:05Z

success.

jaraco added 2 commits September 26, 2019 14:32

Add tox release relying on pep517.

7287089

Rely on tox -e release during mechanical releases.

43ec070

jaraco mentioned this pull request Sep 26, 2019

test_http_over_https_error failing on macOS #225

Open

webknjaz reviewed Sep 26, 2019

View reviewed changes

Rely on environment variable to indicate TOXENV

31150f6

webknjaz reviewed Sep 26, 2019

View reviewed changes

jaraco merged commit c7ca7ff into master Sep 26, 2019

jaraco deleted the feature/pep517-release branch September 26, 2019 20:42

jaraco mentioned this pull request Oct 11, 2019

Use an API token for uploading to PyPI from Travis CI instead of account creds #214

Closed

1 task

webknjaz mentioned this pull request Oct 17, 2019

Project should avoid invoking setup.py #155

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/pep517 release #226

Feature/pep517 release #226

jaraco commented Sep 26, 2019 •

edited by webknjaz

Loading

jaraco commented Sep 26, 2019

webknjaz Sep 26, 2019

jaraco Sep 26, 2019

webknjaz Sep 26, 2019

jaraco Sep 26, 2019

webknjaz Sep 26, 2019

webknjaz Sep 26, 2019

webknjaz Sep 26, 2019

jaraco Sep 26, 2019

webknjaz Sep 26, 2019

jaraco Sep 26, 2019

webknjaz Sep 26, 2019

webknjaz Sep 26, 2019

jaraco Sep 26, 2019

webknjaz Sep 26, 2019

webknjaz commented Sep 26, 2019

codecov bot commented Sep 26, 2019

webknjaz Sep 26, 2019

jaraco Sep 26, 2019

webknjaz Sep 26, 2019

jaraco commented Sep 26, 2019

jaraco commented Sep 26, 2019

webknjaz commented Sep 26, 2019

jaraco commented Sep 26, 2019

	- python -m pip install tox tox-venv
	- python -m pip install tox tox-venv
	- python -m tox --notest

Feature/pep517 release #226

Feature/pep517 release #226

Conversation

jaraco commented Sep 26, 2019 • edited by webknjaz Loading

jaraco commented Sep 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webknjaz commented Sep 26, 2019

codecov bot commented Sep 26, 2019

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaraco commented Sep 26, 2019

jaraco commented Sep 26, 2019

webknjaz commented Sep 26, 2019

jaraco commented Sep 26, 2019

jaraco commented Sep 26, 2019 •

edited by webknjaz

Loading