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

Various catalog fixes #165

Merged
merged 6 commits into from Nov 2, 2017
Merged

Conversation

dopplershift
Copy link
Member

  • Handle RAMADDA TDS catalogs by allowing for no name attribute on catalog refs (just use title)
  • Fix generating access URLs for RAMADDA (which uses a full URL for services) by properly using urljoin
  • Fix catalog reference URL generation on TDS 5 by making sure we use the url from the response as the actual catalog URL (handles forwards)

Just fall back to title if name is missing.
jrleeman
jrleeman previously approved these changes Nov 1, 2017
Copy link
Contributor

@jrleeman jrleeman left a comment

Choose a reason for hiding this comment

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

LGTM pending changes to make 2.7 happy.

The base for services can be a full url with hostname, so use urljoin to
handle joining, which is more robust than bare concatenation anyway.
This fixes creating access urls for RAMADDA.
TDS 5 forwards from /thredds/catalog.xml to
/thredds/catalog/catalog.xml, but we were assuming the catalog given to
the class was the actual URL. Fix that by using the url included in the
response as the actual catalog URL.
Use pytest filtering; before we were accidentally filtering all of the
warnings in the test suite. Also add explicit test of warning in the
catalog html forward test.
Pandas 0.21 started warning about getting the behavior we exactly
expect, so filter out those warnings.
Pass the raw response content, rather than decoded text, to the element
tree parser. This allows the parser to detect the encoding using the
standard xml tags.
Copy link
Contributor

@jrleeman jrleeman left a comment

Choose a reason for hiding this comment

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

Looks good - heck of a maintenance set of changes!

@jrleeman jrleeman merged commit 5f7a89d into Unidata:master Nov 2, 2017
@dopplershift dopplershift deleted the catalog-fixes branch November 2, 2017 15:48
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

2 participants