Oauth support - Githubissues

ohsu-comp-bio / cwl-tes

cwl-tes submits your tasks to a TES server. Task submission is parallelized when possible.

Apache License 2.0

18 stars 28 forks source link

Oauth support #29

Closed uniqueg closed 5 years ago

uniqueg commented 5 years ago

This PR adds --token and --token-public-key to pass OAuth2 Bearer tokens to the TES endpoint. The token is validated via the public key.

This PR replaces #27 by @juhtornr (which was unfortunately never merged). The differences between #27 and this PR are:

the new version is based on the latest/current version of cwl-tes master, so there will be no merge conflicts
it does not include a default public key
it bumps the versions of cwltool and py-tes to some newer versions

The suggested changes have been successfully tested in https://github.com/elixir-europe/WES-ELIXIR and this merge would help us immensely because we wouldn't have to keep up maintaining a fork.

uniqueg commented 5 years ago

@mr-c: Do you have permissions to check what went wrong here (speaking about the failed Travis jobs)? Else, do you know who I could speak to look at this? Thanks!

mr-c commented 5 years ago

@uniqueg Doesn't seem to require any special permissions, see https://travis-ci.org/ohsu-comp-bio/cwl-tes/jobs/579333468#L518

Looks like formatting errors?

https://travis-ci.org/ohsu-comp-bio/cwl-tes/jobs/579333468#L582 is more serious. Maybe you will need https://docs.travis-ci.com/user/running-build-in-debug-mode/

uniqueg commented 5 years ago

Well, something weird there, because at least most of the linter errors affect lines and files that have not been changed for this PR (e.g., typing declarations). I would venture to say that the current master branch also would not pass these. And possibly run into the same FileNotFound error as well...

mr-c commented 5 years ago

My guess is that there is a new pylint (or similar) version since the last time there was a PR. You could pin it to the older version as part of this PR

adamstruck commented 5 years ago

The PR looks good otherwise and addresses all concerns from #27.

@uniqueg I am fine with merging this in given that the tests don't pass on the master branch either, but will hold off if you are already looking into making the changes to get these tests to pass.

adamstruck commented 5 years ago

flake8 passes with version 3.6.0, but fails for versions >= 3.7.0

uniqueg commented 5 years ago

Thanks @adamstruck! I am not very strong on Travis CI, and since I'm quite busy atm, I had put this off. Would really appreciate the merge.