search

OpenTreeOfLife / phylesystem-api

API access to Open Tree of Life treestore
BSD 2-Clause "Simplified" License
10 stars 5 forks source link

Fix and simplify api urls #177

Closed jimallman closed 8 years ago

jimallman commented 8 years ago

Support the new use of domain-only base URLs in system-config files. To do this, we tweak the internal definition of some API URLs to use the older, versioned URLs. This also modifies the on-commit webhook on GitHub.

Addresses #176. These changes are running now on devtree.

IMPORTANT: Once this PR is merged to master, we'll need to update the production api config file to match the domain-only base URLs used in devapi.

jar398 commented 8 years ago

re the 'IMPORTANT', since I'll probably be the one doing this, can you put appropriate instructions in the config file itself? and what will be the failure if I forget?

jimallman commented 8 years ago

Compare the two linked config files (highlighted lines) to see the difference in *-BASE_URL values; these should omit the slash and path following the hostname, for example

OPENTREE_API_BASE_URL=https://${OPENTREE_API_HOST}/phylesystem/v1

should become 

OPENTREE_API_BASE_URL=https://${OPENTREE_API_HOST}

Probably the simplest thing I can do is to duplicate these lines in the production file, make the desired changes, and comment them out.

jimallman commented 8 years ago

On second thought, the simplest thing is a (rare) pull request in deployed-systems, which should be merged only after this one.

jimallman commented 8 years ago

what will be the failure if I forget?

Good question. Some of these failures here can be pretty subtle, but the best "tell" will be lagging changes to studies in the index (oti).

@jar398, how would you recommend running the unit tests in a way that takes into account the production config? The steps in TESTING.md seem a bit involved (installing peyotl, cloning repos, venv?) and not safe to do even on devapi. (Or am I mistaken, and I should simply run them on devapi within our usual venv?)

jar398 commented 8 years ago

Which TESTING.md are you talking about, germinator or phylesystem-api? Not sure what you mean by not safe to run, as far as I know the tests are safe, when invoked from run_tests.sh in the germinator repo. For phylesystem-api you have to supply the right config parameter to suppress the tests that write phylesystem, but the germinator script does this.

I suppose we could collect all the tests in the germinator repo, so that only it would have to be checked out, but there's an obvious tension between modularity and dependency reduction.

I don't know about the peyotl dependency; it could be unnecessary, or easy to work around.

Anyhow I can (& probably will) run the tests when I do the update, I usually do. So you don't need to worry about it.

On Mon, May 9, 2016 at 7:19 PM, Jim Allman notifications@github.com wrote:

what will be the failure if I forget?

Good question. Some of these failures here can be pretty subtle, but the best "tell" will be lagging changes to studies in the index (oti).

@jar398 https://github.com/jar398, how would you recommend running the unit tests in a way that takes into account the production config? The steps in TESTING.md seem a bit involved (installing peyotl, cloning repos, venv?) and not safe to do even on devapi. (Or am I mistaken, and I should simply run them on devapi within our usual venv?)

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/OpenTreeOfLife/phylesystem-api/pull/177#issuecomment-218017854

jimallman commented 8 years ago

Which TESTING.md are you talking about, germinator or phylesystem-api?

Sorry, I meant the one in phlesystem-api. (I added a working hyperlink in a later edit.)

Anyhow I can (& probably will) run the tests when I do the update, I usually do. So you don't need to worry about it.

Thanks! I tried to test each URL in the system as it was changed.