From cf87bf8a26c7a22e9590c5393415d616d8d7afa4 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sat, 7 Dec 2019 14:10:17 +0100 Subject: [PATCH 1/7] Refresh .travis.yml: test py3.5-3.8, CD to PyPI --- .travis.yml | 30 ++++++++++++++++++++++-------- dev-requirements.txt | 1 + 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1998d36..46cb02f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,17 +1,31 @@ +dist: bionic language: python -sudo: false python: + - 3.8 + - 3.7 - 3.6 - 3.5 -# install dependencies +# initial step - install dependencies install: - - pip install --no-cache-dir -e . - - pip install --no-cache-dir -r dev-requirements.txt + - pip install --upgrade pip + - pip install -r dev-requirements.txt + - pip install . + - pip freeze -# command to run tests +# middle step - run tests script: - - py.test tests/ + - pytest --verbose --flakes -matrix: - fast_finish: true +# final step - optionally deploy +deploy: + provider: pypi + user: "__token__" + # password: set by TravisCI's environment variable PYPI_PASSWORD + # ref: https://travis-ci.org/jupyterhub/ltiauthenticator/settings + distributions: sdist bdist_wheel + on: + # Only deploy on tagged commits instead of the default of only doing it to + # the master branch. A tag does not belong specifically to a branch, so + # without this it would fail to deploy for tags. + tags: true diff --git a/dev-requirements.txt b/dev-requirements.txt index e079f8a..ed724d6 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1 +1,2 @@ pytest +pytest-flakes From a94eebaabbfd7c2b77773198452444526503d116 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sat, 7 Dec 2019 14:44:34 +0100 Subject: [PATCH 2/7] Unused var "user" now used - further investigation essential References: - Class dependencies: - jupyterhub.handlers.BaseHandler: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L69 - tornado.web.RequestHandler: https://www.tornadoweb.org/en/stable/web.html#tornado.web.RequestHandler - Function dependencies: - login_user: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L696-L715 - get_next_url: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L587 - get_body_argument: https://www.tornadoweb.org/en/stable/web.html#tornado.web.RequestHandler.get_body_argument - Function overrided: jupyterhub.handlers.BaseHandler.post: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L1289-L1313 Needs verification: 1. The user object was not consumed, perhaps it should be passed to the get_next_url function but wasn't. I made it so for now. 2. I also named parameters passed to get_body_argument to make it easier to read what it is about. 3. Is it correct that this function signature should only be (self)? 4. Is it correct that this function should not be decorated with something like @web.authenticated like the other BaseHandler functions like get(self, user_name, user_path) is? --- ltiauthenticator/__init__.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/ltiauthenticator/__init__.py b/ltiauthenticator/__init__.py index b745b2e..bc61f19 100644 --- a/ltiauthenticator/__init__.py +++ b/ltiauthenticator/__init__.py @@ -184,5 +184,19 @@ class LTIAuthenticateHandler(BaseHandler): @gen.coroutine def post(self): + # References: + # - Class dependencies: + # - jupyterhub.handlers.BaseHandler: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L69 + # - tornado.web.RequestHandler: https://www.tornadoweb.org/en/stable/web.html#tornado.web.RequestHandler + # - Function dependencies: + # - login_user: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L696-L715 + # - get_next_url: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L587 + # - get_body_argument: https://www.tornadoweb.org/en/stable/web.html#tornado.web.RequestHandler.get_body_argument user = yield self.login_user() - self.redirect(self.get_body_argument('custom_next', self.get_next_url())) + next_url = self.get_next_url(user=user) + body_argument = self.get_body_argument( + name='custom_next', + default=next_url, + ) + + self.redirect(body_argument) From b31d0c36d44a32b9b545998b4f122bb910da1969 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sat, 7 Dec 2019 14:56:38 +0100 Subject: [PATCH 3/7] Removed unused import --- ltiauthenticator/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ltiauthenticator/__init__.py b/ltiauthenticator/__init__.py index bc61f19..9d1f74e 100644 --- a/ltiauthenticator/__init__.py +++ b/ltiauthenticator/__init__.py @@ -1,6 +1,6 @@ import time -from traitlets import Bool, Dict +from traitlets import Dict from tornado import gen, web from jupyterhub.auth import Authenticator From e1a2c0d0ebc2665c63d412393723b377fcf45ec7 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sat, 7 Dec 2019 15:00:41 +0100 Subject: [PATCH 4/7] Add __version__ and bump2version --- .bumpversion.cfg | 21 +++++++++++++++++++++ dev-requirements.txt | 1 + ltiauthenticator/__init__.py | 2 ++ setup.py | 2 +- 4 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 .bumpversion.cfg diff --git a/.bumpversion.cfg b/.bumpversion.cfg new file mode 100644 index 0000000..05c6e97 --- /dev/null +++ b/.bumpversion.cfg @@ -0,0 +1,21 @@ +[bumpversion] +current_version = 0.4.0.dev +parse = (?P\d+)\.(?P\d+)\.(?P\d+)(\.(?P[a-z0-9]+))? +tag_name = {new_version} +allow_dirty = True +commit = True +tag = False +serialize = + {major}.{minor}.{patch}.{release} + {major}.{minor}.{patch} + +[bumpversion:file:ltiauthenticator/__init__.py] + +[bumpversion:file:setup.py] + +[bumpversion:part:release] +optional_value = stable +values = + dev + stable + diff --git a/dev-requirements.txt b/dev-requirements.txt index ed724d6..c44a964 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,2 +1,3 @@ +bump2version pytest pytest-flakes diff --git a/ltiauthenticator/__init__.py b/ltiauthenticator/__init__.py index 9d1f74e..9c40786 100644 --- a/ltiauthenticator/__init__.py +++ b/ltiauthenticator/__init__.py @@ -10,6 +10,8 @@ from oauthlib.oauth1.rfc5849 import signature from collections import OrderedDict +__version__ = '0.4.0.dev' + class LTILaunchValidator: # Record time when process starts, so we can reject requests made # before this diff --git a/setup.py b/setup.py index ed65f1a..d9e4969 100644 --- a/setup.py +++ b/setup.py @@ -2,7 +2,7 @@ setup( name='jupyterhub-ltiauthenticator', - version='0.3', + version='0.4.0.dev', description='JupyterHub authenticator implementing LTI v1', url='https://github.com/yuvipanda/jupyterhub-ltiauthenticator', author='Yuvi Panda', From 1dd9dc46ef2cd87f17784bc5070423a85c55cb07 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sat, 7 Dec 2019 15:04:00 +0100 Subject: [PATCH 5/7] Add a RELEASE.md --- RELEASE.md | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 RELEASE.md diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 0000000..55f7ce7 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,71 @@ +# How to make a release + +`jupyterhub-ltiauthenticator` is a package [available on +PyPI](https://pypi.org/project/jupyterhub-ltiauthenticator/). These are +instructions on how to make a release on PyPI. The PyPI release is packaged and +published automatically by TravisCI when a git tag is pushed. + +For you to follow along according to these instructions, you need: +- To have push rights to the [ltiauthenticator GitHub + repository](https://github.com/jupyterhub/ltiauthenticator). + +## Steps to make a release + +1. Update [CHANGELOG.md](CHANGELOG.md) if it is not up to date, and verify + [README.md](README.md) has an updated output of running `--help`. Make a PR + to review the CHANGELOG notes. + + To get the foundation of the changelog written, you can install + [github-activity](https://github.com/choldgraf/github-activity) and run + `github-activity --kind pr jupyterhub/ltiauthenticator` after setting up + credentials as described in the project's README.md file. + +1. Once the changelog is up to date, checkout master and make sure it is up to date and clean. + + ```bash + ORIGIN=${ORIGIN:-origin} # set to the canonical remote, e.g. 'upstream' if 'origin' is not the official repo + git checkout master + git fetch $ORIGIN master + git reset --hard $ORIGIN/master + # WARNING! This next command deletes any untracked files in the repo + git clean -xfd + ``` + +1. Update the version with `bump2version` (can be installed with `pip install -r + dev-requirements.txt`) + + ```bash + VERSION=... # e.g. 1.2.3 + bump2version --tag --new-version $VERSION - + ``` + +1. Reset the version to the next development version with `bump2version` + + ```bash + bump2version --no-tag patch + ``` + +1. Push your two commits to master along with the annotated tags referencing + commits on master. + + ``` + git push --follow-tags $ORIGIN master + ``` + +## Manually uploading to PyPI + +We are using CD with Travis to automatically update PyPI, but if you want to do +it manually when you are on a tagged commit in a otherwise cleaned repository, +you can do this. + +1. Package the release + + ```bash + python3 setup.py sdist bdist_wheel + ``` + +1. Upload it to PyPI + + ```bash + twine upload dist/* + ``` From 6e8a4b285b521ba2b437b4f3cbb3a37cceb13264 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Sun, 8 Dec 2019 12:50:27 +0100 Subject: [PATCH 6/7] Adjust inline documentation --- ltiauthenticator/__init__.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/ltiauthenticator/__init__.py b/ltiauthenticator/__init__.py index 9c40786..df832f0 100644 --- a/ltiauthenticator/__init__.py +++ b/ltiauthenticator/__init__.py @@ -186,14 +186,21 @@ class LTIAuthenticateHandler(BaseHandler): @gen.coroutine def post(self): - # References: - # - Class dependencies: - # - jupyterhub.handlers.BaseHandler: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L69 - # - tornado.web.RequestHandler: https://www.tornadoweb.org/en/stable/web.html#tornado.web.RequestHandler - # - Function dependencies: - # - login_user: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L696-L715 - # - get_next_url: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L587 - # - get_body_argument: https://www.tornadoweb.org/en/stable/web.html#tornado.web.RequestHandler.get_body_argument + """ + Technical reference + ------------------- + 1. Class dependencies + - jupyterhub.handlers.BaseHandler: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L69 + - tornado.web.RequestHandler: https://www.tornadoweb.org/en/stable/web.html#tornado.web.RequestHandler + 2. Function dependencies + - login_user: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L696-L715 + login_user is defined in the JupyterHub wide BaseHandler class, + mainly wraps a call to the authenticate function and follow up. + a successful authentication with a call to auth_to_user that + persists a JupyterHub user and returns it. + - get_next_url: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L587 + - get_body_argument: https://www.tornadoweb.org/en/stable/web.html#tornado.web.RequestHandler.get_body_argument + """ user = yield self.login_user() next_url = self.get_next_url(user=user) body_argument = self.get_body_argument( From 6317dc80980159d6474d0cd2154db43aca098d12 Mon Sep 17 00:00:00 2001 From: Erik Sundell Date: Mon, 9 Dec 2019 09:20:56 +0100 Subject: [PATCH 7/7] Revert not well understood potential fix Passing the user returned from self.login_user() to self.get_next_url() can perhaps impact the value returned by self.get_next_url in a way that fixes an issue. This is not obvious to me if it will or not, it just seems likely that it makes sense to do. Without further information and experimentation, I'll revert doing that as it could lead to a breaking change for users. --- ltiauthenticator/__init__.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ltiauthenticator/__init__.py b/ltiauthenticator/__init__.py index df832f0..b4317e1 100644 --- a/ltiauthenticator/__init__.py +++ b/ltiauthenticator/__init__.py @@ -187,8 +187,8 @@ class LTIAuthenticateHandler(BaseHandler): @gen.coroutine def post(self): """ - Technical reference - ------------------- + Technical reference of relevance to understand this function + ------------------------------------------------------------ 1. Class dependencies - jupyterhub.handlers.BaseHandler: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L69 - tornado.web.RequestHandler: https://www.tornadoweb.org/en/stable/web.html#tornado.web.RequestHandler @@ -201,8 +201,12 @@ def post(self): - get_next_url: https://github.com/jupyterhub/jupyterhub/blob/abb93ad799865a4b27f677e126ab917241e1af72/jupyterhub/handlers/base.py#L587 - get_body_argument: https://www.tornadoweb.org/en/stable/web.html#tornado.web.RequestHandler.get_body_argument """ - user = yield self.login_user() - next_url = self.get_next_url(user=user) + # FIXME: Figure out if we want to pass the user returned from + # self.login_user() to self.get_next_url(). It is named + # _ for now as pyflakes is fine about having an unused + # variable named _. + _ = yield self.login_user() + next_url = self.get_next_url() body_argument = self.get_body_argument( name='custom_next', default=next_url,