biocore / redbiom

Sample search by metadata and features
Other
45 stars 20 forks source link

Release fixes #37

Closed wasade closed 7 years ago

wasade commented 7 years ago

I think this covers the necessary changes. Do not merge until we're green on travis as this PR introduces a default non-localhost REDBIOM_HOST necessitating logic to complain appropriately if testing off localhost.

wasade commented 7 years ago

Verify that the default host that is read only (qiita) works, and then switch to a host which is mutable so we can execute the test suite

On Jun 7, 2017 05:06, "Antonio Gonzalez" notifications@github.com wrote:

@antgonza commented on this pull request.

looks good, 1 question.

In .travis.yml https://github.com/biocore/redbiom/pull/37#discussion_r120606566:

@@ -26,5 +26,7 @@ install:

  • alias md5=md5sum
  • pip install -e . --no-deps script:
    • flake8 --ignore=E731 redbiom
    • flake8 --ignore=E731 redbiom
    • redbiom summarize contexts # will return a nonzero exit status if it cannot communicate with the default host

I don't get this, you are testing if it can communicate (which it will fail and stop execution) and then change REDBIOM_HOST? Would you mind explaining why is this required?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/biocore/redbiom/pull/37#pullrequestreview-42581752, or mute the thread https://github.com/notifications/unsubscribe-auth/AAc8sgFeciyhZngRs6XvGIhEuBpIpmVDks5sBpIpgaJpZM4NyGQs .

wasade commented 7 years ago

Yes. But that shouldn't happen, right?

On Jun 7, 2017 7:12 AM, "Antonio Gonzalez" notifications@github.com wrote:

@antgonza commented on this pull request.

In .travis.yml https://github.com/biocore/redbiom/pull/37#discussion_r120636258:

@@ -26,5 +26,7 @@ install:

  • alias md5=md5sum
  • pip install -e . --no-deps script:
    • flake8 --ignore=E731 redbiom
    • flake8 --ignore=E731 redbiom
    • redbiom summarize contexts # will return a nonzero exit status if it cannot communicate with the default host

just to confirm, does this mean that it will fail if qiita is down?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/biocore/redbiom/pull/37#discussion_r120636258, or mute the thread https://github.com/notifications/unsubscribe-auth/AAc8snjMwC14x-H2TELpfY_MP4SOBaZFks5sBq-_gaJpZM4NyGQs .

wasade commented 7 years ago

Thanks!

On Jun 7, 2017 7:30 AM, "Antonio Gonzalez" notifications@github.com wrote:

Merged #37 https://github.com/biocore/redbiom/pull/37.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/biocore/redbiom/pull/37#event-1113768291, or mute the thread https://github.com/notifications/unsubscribe-auth/AAc8st6_TaKtY8NHOag4SPRumV9XgOuhks5sBrPsgaJpZM4NyGQs .