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

Fix Travis-CI #49

Merged
merged 4 commits into from
Nov 17, 2015
Merged

Fix Travis-CI #49

merged 4 commits into from
Nov 17, 2015

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Nov 15, 2015

In order to investigate SECOORA/skill_score#225 I am coming back to pyoos a lot, so we need to get Travis-CI back online.

  • pyoos builds on Python 3, but is far from py3k compliant. There is a significant amount of work to do. I am deleting the pyoos python3 binaries from the ioos channel.
  • Some of the failures we are seeing are in the Python 2.7 are similar to what I am observing in my SECOORA notebooks. I will keep investigating what changed.

Ping @kwilcox, @rsignell-usgs, and @vembus.

import sys

from setuptools import setup, find_packages
from setuptools.command.test import test as TestCommand

from pyoos import __version__
def extract_version(module='pyoos'):
Copy link
Member

Choose a reason for hiding this comment

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

i've seen this pattern used somewhere else - can you explain what it's accomplishing and why it's better than the simpler import version?

Copy link
Member Author

Choose a reason for hiding this comment

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

This pattern avoids importing the module before installing it. It does add some complexity, but it helps some packaging tools. For example: if this is not there we need to install all the dependencies at build time, which is not always desirable.

However, that is not essential and not the main point of this PR. I can remove that if you wish.

@ocefpaf ocefpaf mentioned this pull request Nov 17, 2015
@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 17, 2015

Fix #36

@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 17, 2015

The test_coops_describe_sensor failure (https://travis-ci.org/ioos/pyoos/jobs/91279614#L355) seems related to this: SECOORA/skill_score#225 (comment).

And maybe the test_coops_server_id (https://travis-ci.org/ioos/pyoos/jobs/91279614#L356) is related to this: SECOORA/skill_score#225 (comment)

@ocefpaf ocefpaf force-pushed the fix_travis branch 2 times, most recently from f31b287 to 46c5bc8 Compare November 17, 2015 18:55
@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 17, 2015

I fixed all but Nerrs failures. @emiliom Are you familiar with Nerrs? Does your PR #48 address some of these issues?

See https://travis-ci.org/ioos/pyoos/jobs/91659664#L372-L378

@emiliom
Copy link
Contributor

emiliom commented Nov 17, 2015

@ocefpaf, I'm very familiar with Nerrs -- that's why I submitted PR #48 two weeks ago! Are you a pyoos maintainer now?? Thanks for following up. FYI, I'm using my pyoos NERRS fork updates (that are in this PR) operationally on an application, pulling data every hour or so.

Unfortunately I can't follow up right now b/c I'm still in meetings. I have a hunch as to what could be causing failures in automatic tests, but I don't have time right now to follow up. Maybe Wednesday evening ...

@daf
Copy link
Member

daf commented Nov 17, 2015

I'm happy to merge this without it fully passing and addressing NERRS separately, @ocefpaf .

@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 17, 2015

Are you a pyoos maintainer now??

Nope. Just part of the cleaning crew.

Maybe Wednesday evening

No problem. Maybe we can merge this as is (@kwilcox or @daf ?). Later on and you can rebase your branch to see if the NERRS tests passes or not.

daf added a commit that referenced this pull request Nov 17, 2015
@daf daf merged commit ea90470 into ioos:master Nov 17, 2015
@daf
Copy link
Member

daf commented Nov 17, 2015

Sounds good!

@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 17, 2015

Thanks @daf you were faster than I 😉

@ocefpaf ocefpaf deleted the fix_travis branch November 17, 2015 19:34
@ocefpaf ocefpaf mentioned this pull request Nov 17, 2015
@kwilcox
Copy link
Member

kwilcox commented Nov 17, 2015

NERRS tests probably fail because the code assumes they allow your IP address to access their service. Emilio's PR addresses this by adding the token based auth. Someone will need to get a token from NERRS to use in testing.

@emiliom
Copy link
Contributor

emiliom commented Nov 17, 2015

@kwilcox:

NERRS tests probably fail because the code assumes they allow your IP address to access their service.

That was exactly my hunch.

Emilio's PR addresses this by adding the token based auth. Someone will need to get a token from NERRS to use in testing.

I have a token, but I don't feel at liberty to place it in a publicly visible location. FYI, that access option was developed by NERRS/CDMO last year partly through my nudging. Last time I asked them (last year), they were not comfortable opening it up for wider use. I/we should ask/encourage them again ...

@emiliom
Copy link
Contributor

emiliom commented Nov 17, 2015

@ocefpaf:

Maybe we can merge this as is (@kwilcox or @daf ?). Later on and you can rebase your branch to see if the NERRS tests passes or not.

Looks like it's merged already, right? Cool! I can rebase on my end next week. But the testing will still be tricky, b/c of the issue @kwilcox mentioned, and b/c I'm reluctant to place my auth token in a publicly visible location w/o NERRS/CDMO's blessing.

@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 17, 2015

Maybe we should add a known failure or skip flag to those tests. @kwilcox thoughts?

@kwilcox
Copy link
Member

kwilcox commented Nov 23, 2015

@emiliom Since you have connections with the CDMO group already, would you mind contacting them and asking for a token to be used in testing only (very limited usage). We can encrypt it into the .travis.yml to prevent abuse.

Other than that... we need someone who is wiling to port pyoos to py3!

@emiliom
Copy link
Contributor

emiliom commented Nov 23, 2015

@emiliom Since you have connections with the CDMO group already, would you mind contacting them and asking for a token to be used in testing only (very limited usage).

Will do.

We can encrypt it into the .travis.yml to prevent abuse.

Ah, excellent! That will address the concern.

Other than that... we need someone who is wiling to port pyoos to py3!

Heh. Not me. First I have to start using py3 ...

@ocefpaf
Copy link
Member Author

ocefpaf commented Dec 28, 2015

Other than that... we need someone who is wiling to port pyoos to py3!

See #52

@lukecampbell
Copy link
Member

👍

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.

None yet

5 participants