Skip to content
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

Add travis #50

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open

Add travis #50

wants to merge 16 commits into from

Conversation

kdaily
Copy link
Contributor

@kdaily kdaily commented Jan 31, 2018

to run tests

@kdaily kdaily self-assigned this Jan 31, 2018
@kdaily
Copy link
Contributor Author

kdaily commented Jan 31, 2018

This is blocked by https://sagebionetworks.jira.com/browse/SYNPY-639

Copy link
Contributor

@sgosline sgosline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once the jira is fixed this looks good to me.

@teslajoy teslajoy removed their request for review January 31, 2018 22:16
@philerooski
Copy link
Collaborator

I've been using pytest to run my unit tests. Are there going to be any compatibility issues?

@kdaily
Copy link
Contributor Author

kdaily commented Jan 31, 2018

Short answer: yes.

@kdaily
Copy link
Contributor Author

kdaily commented Jan 31, 2018

We should use pytest anyways, nose hasn't been maintained.

@kdaily
Copy link
Contributor Author

kdaily commented Jan 31, 2018

@sgosline see #51

Kenneth Daily and others added 10 commits January 31, 2018 15:21
use pytest instead of nose
add nose and pytest as dependencies
install myself
changed setuppy + path to test csv files (should pull from synapse in future) + closes #51
add requests
install requirements first, duh
@kdaily
Copy link
Contributor Author

kdaily commented Mar 2, 2018

@philerooski @teslajoy any idea why this would be failing getting the version number from github (but only when running on Travis)?

https://travis-ci.org/Sage-Bionetworks/annotator/jobs/348357181

@teslajoy
Copy link
Collaborator

teslajoy commented Mar 2, 2018

Not sure, but a couple of brainstorming-thoughts on the travis env:

I'd probably try to remove pip install -r ... and leave pip install . (maybe some library dependency stack is being corrupted)
I'd also try to get a pip install . --verbose to see a bit more of the traceback
If non of this worked, I'd try to change the env.

@kdaily
Copy link
Contributor Author

kdaily commented Mar 2, 2018

need the pip install -r requirements.txt because setup.py has requirements (including requests which is causing the error).

@kdaily
Copy link
Contributor Author

kdaily commented Mar 2, 2018

Ran it a few more times, finally figured it out:

{u'documentation_url': u'https://developer.github.com/v3/#rate-limiting', u'message': u"API rate limit exceeded for 52.3.55.28. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)"}

We need to use a Github API Token in our request in order to guarantee this call will work.

@kdaily
Copy link
Contributor Author

kdaily commented Mar 2, 2018

Looking further, we might not be able to do it this way because to supply the Github auth token as an environment variable (standard way) won't work on Travis for PR builds:

https://docs.travis-ci.com/user/pull-requests#Pull-Requests-and-Security-Restrictions

@teslajoy
Copy link
Collaborator

teslajoy commented Mar 2, 2018

I added TSD_GITHUB_TOKEN to travis via:
https://www.tomsdev.com/blog/2015/tsd-travis-ci-github-rate-limit-reached/

would have to add it to global env and in code if it exists add to auth of requests.get() else just leave request as is for local users env.
http://docs.python-requests.org/en/master/user/authentication/
https://docs.travis-ci.com/user/environment-variables/

@kdaily
Copy link
Contributor Author

kdaily commented Mar 2, 2018

After contemplating what this code is doing, not sure it's the best way to get the version!

This PR is a good example - when I install, I'm going to report that my version is whatever is the latest version in Github - but that isn't true! This code is newer than the most recent version. We need to manage the state of versions locally (in this codebase) or through git (not Github) tags, not based on previous releases.

In order to get this in, I'm going to make another issue/PR to remove and change the versioning process first.

@kdaily
Copy link
Contributor Author

kdaily commented Mar 2, 2018

Blocked by #88.

@teslajoy
Copy link
Collaborator

teslajoy commented Mar 2, 2018

Agreed! Great idea as always!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants