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
Migrate oauth to GitHub app #155
Conversation
… into phylesystemapi
This reverts commit a74348e.
merging easter-fix into migrate-oauth-to-github-app for full testing on dev* servers
This prevents errors when (e.g.) sending a CORS preflight 'OPTONS' request to fetch cached API responses. Test: If this patch is not applied, no tree will appear in our synth-tree viewer webapp.
This prevents errors when authorizing to a GitHub app (or OAuth app) on proxied web2py servers, as is common in our setup. Test: If this patch is not applied, we'll be unable to successfully login to the synth-tree viewer or curation app. (This error usually manifests as a mismatched-uri, even when the requested and registered URLs are identical.)
Happily newer versions of web2py correctly handle non-ASCII names when importing a python module.
rsync -p -e "${SSH}" "$OPENTREE_GH_IDENTITY" "$OT_USER@$OPENTREE_HOST":.ssh/opentree | ||
${SSH} "$OT_USER@$OPENTREE_HOST" chmod 600 .ssh/opentree | ||
rsync -p -e "${SSH}" "$OPENTREE_GH_IDENTITY" "$OT_USER@$OPENTREE_HOST":.ssh/opentree || exit 1 | ||
${SSH} "$OT_USER@$OPENTREE_HOST" chmod 600 .ssh/opentree || exit 1 | ||
fi | ||
|
||
# Try to place an OAuth token for GitHub API by bot user 'opentreeapi' | ||
tokenfile=${OPENTREE_SECRETS}/OPENTREEAPI_OAUTH_TOKEN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused about what should be happening here. I should have the OPENTREE_OAUTH_TOKEN saved in .ssh/opentree/ on my local machine (https://github.com/OpenTreeOfLife/germinator/pull/155/files#diff-028ee22cd145bc1227616bfad2602bdeR90), and then it gets copied over? Is this how it should still work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - following conversation with Mark I believe we are using a keypair generated directly on phylsystemapi/devphylesystemapi for the pushes to phylesystem. So I think we don't need this bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm it looks like it also tries to grab the opentree key here https://github.com/OpenTreeOfLife/germinator/pull/155/files#diff-028ee22cd145bc1227616bfad2602bdeR410
|
||
|
||
|
||
|
||
# ---------- Redis for caching --------- | ||
#This is in the default venv | ||
REDIS_WITH_VERSION="redis-2.10.6" | ||
REDIS_WITH_VERSION="redis-3.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made this change, but based on experiments on devphylesystemapi needs to be reverted to redis==2.10.6. I don't recollect why I changed it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So are we running two versions of redis, one inside and one outside of the python virtual environment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A good q.. and one that I don't honestly understand the answer to!
on dev phylsesystem-api redis-cli 3.0.0 is installed from releases, but we were running into celery/celery#5175 until @mtholder ran venv/bin/pip install redis==2.10.6
But AFAIK redis 3.0.0 is still getting called by https://github.com/OpenTreeOfLife/germinator/blob/migrate-oauth-to-github-app/deploy/setup/install-api.sh#L264 I may be wrong about that part though! Mark might be able to clarify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what is running. Here is what I suspect: the redis server can probably be running 2.10.6 or 3.0.0.
However, celery has be running through python2.7 (because we are using web2py). It imports the redis python package to interact with the redis server. I think that the means that we need to be using a not-the-latest version of the redis python package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah right - that sounds correct to me!
OK! I re-deployed devphylesystemapi with this branch, and all seems good! Currently requires deployer to place opentree-api.pem and set git config, but can make a new branch to add that later once this PR is merged. |
Sure, I'll take a look... |
As it turns out, there was some non-trivial picking and choosing here. I kept the switch from |
This branch also incorporates selected improvements from
easter-fix
, all now conforming to our usual deployment methods (no manual tweaking required).