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 ESO #970

Merged
merged 7 commits into from Sep 8, 2017
Merged

Fix ESO #970

merged 7 commits into from Sep 8, 2017

Conversation

keflavich
Copy link
Contributor

@keflavich keflavich commented Sep 6, 2017

ESO queries are broken (see #969). We need to fix the module.

This is a WIP that fixes login. The todo items are:

  • list_instruments
  • list_surveys
  • query_apex_quicklooks
  • query_instrument
  • query_survey
  • query_surveys
  • retrieve_data

@pep8speaks
Copy link

pep8speaks commented Sep 6, 2017

Hello @keflavich! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on September 08, 2017 at 17:35 Hours UTC

@astropy-bot
Copy link

astropy-bot bot commented Sep 6, 2017

Hi there @keflavich 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labelled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃.

Everything looks good from my point of view! 👍

@bsipocz bsipocz added this to the v0.3.7 milestone Sep 7, 2017
@keflavich keflavich self-assigned this Sep 7, 2017
@keflavich
Copy link
Contributor Author

the conda-forge build appears to be failing. Beyond that, I think this is ready. @bsipocz, any thoughts abouts about the erroring build?

@bsipocz
Copy link
Member

bsipocz commented Sep 8, 2017

@keflavich - Travis has had issues with the osx builds for a few days now, I guess the failing build was part of that issue. I've restarted it now.

@keflavich
Copy link
Contributor Author

@bsipocz I restarted it again, but it looked like maybe you had canceled? I'm ready to merge this PR unless you see any problems. The coverage decrease is fine and, afaict, the appveyor failure is some installation junk that is officially above my pay grade. I think I actually fixed the astropy-bot comments....

@bsipocz
Copy link
Member

bsipocz commented Sep 8, 2017

Travis totally shut down their osx infra, so I vote for merging this without that build (and yes, I've cancelled it again, so it's not blocking other astropy builds in the queue).

The bot complains about the missing changelog, but here it's not an issue at all do to at release time.

@bsipocz
Copy link
Member

bsipocz commented Sep 8, 2017

Hmm, the astropy-helpers change is definitely unneeded, would you remove that last commit?

@keflavich
Copy link
Contributor Author

yeah, that seemed weird to me. Done.

@bsipocz
Copy link
Member

bsipocz commented Sep 8, 2017

Thanks! We can then just merge away, as everything except the osx has passed previously. Or are you waiting for the appveyor build?

@keflavich
Copy link
Contributor Author

I'm OK to merge, but I also am not feeling all that impatient. I can wait for the builds to finish.

@bsipocz
Copy link
Member

bsipocz commented Sep 8, 2017

I'm pretty sure the appveyor issue can be traced back to this:
pyca/pyopenssl#674

I'll try to open a PR with a workaround, but first want to get the aastropy release out of the door.

@keflavich keflavich merged commit 22ec8d2 into astropy:master Sep 8, 2017
@keflavich keflavich deleted the eso_sep2017 branch September 8, 2017 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants