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

Support indexing the ISO files from the IOOS Catalog #574

Merged
merged 9 commits into from
Aug 27, 2018

Conversation

kwilcox
Copy link
Member

@kwilcox kwilcox commented Aug 22, 2018

Overview

I was attempting to setup a pycsw server for IOOS and ran across a few hiccups that I fixed in this PR. I also fixed an outstanding issue (#561) that I ran into when running the functional tests.

Related Issue / Discussion

In general: it was not easy to debug loading a very large folder of ISO files into pycsw. The loops would error out if a file couldn't be parsed and the error messages were not specific enough to debug which files were cause the issues. In some cases the error messages were much to verbose because they contained the entire SQL INSERT statement. The logging when loading records is much more user friendly now. While making the logging more friendly I came across a few other hiccups that I fixed along the way.

Additional Information

Contributions and Licensing

(as per https://github.com/geopython/pycsw/blob/master/CONTRIBUTING.rst#contributions-and-licensing)

  • I'd like to contribute these commits to pycsw. I confirm that my contributions to pycsw will be compatible with the pycsw license guidelines at the time of contribution.
  • I have already previously agreed to the pycsw Contributions and Licensing Guidelines

@codecov
Copy link

codecov bot commented Aug 22, 2018

Codecov Report

Merging #574 into master will increase coverage by 0.59%.
The diff coverage is 62.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #574      +/-   ##
==========================================
+ Coverage   55.81%   56.41%   +0.59%     
==========================================
  Files          28       28              
  Lines        6355     6401      +46     
  Branches     1349     1357       +8     
==========================================
+ Hits         3547     3611      +64     
+ Misses       2422     2401      -21     
- Partials      386      389       +3
Flag Coverage Δ
#integrationtests 55.06% <62.9%> (+0.6%) ⬆️
#unittests 7.62% <9.67%> (+0.03%) ⬆️
Impacted Files Coverage Δ
pycsw/ogc/csw/csw2.py 71.73% <ø> (ø) ⬆️
pycsw/ogc/csw/csw3.py 47.5% <ø> (ø) ⬆️
pycsw/core/repository.py 62.32% <100%> (+1.48%) ⬆️
pycsw/core/metadata.py 22.42% <20%> (+0.11%) ⬆️
pycsw/core/admin.py 38.43% <58.97%> (+13.95%) ⬆️
pycsw/core/util.py 89.28% <82.35%> (-0.96%) ⬇️
pycsw/ogc/fes/fes2.py 39.13% <0%> (ø) ⬆️
pycsw/server.py 64.56% <0%> (+0.2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d008973...9343f08. Read the comment docs.

@kwilcox kwilcox changed the title Support indexing the ISO files from the IOOS Catalog WIP: Support indexing the ISO files from the IOOS Catalog Aug 22, 2018
@kwilcox
Copy link
Member Author

kwilcox commented Aug 22, 2018

I'm running the tests against sqlite and postgres to make sure the VACUUM fix is actually fixed...

Carry through the actual exception when trying to insert records
This turns

```
<ows:ExceptionText>Harvest failed: record parsing failed: local variable 'md' referenced before assignment</ows:ExceptionText>
```

into

```
<ows:ExceptionText>Harvest failed: record parsing failed: 404 Client Error: Not Found for url: http://maps.cera.govt.nz/arcgis/services/CERA/CERA_TechClasses_WGS84/MapServer/WFSServer?service=WFS&amp;request=GetCapabilities&amp;version=2.0.0</ows:ExceptionText>
```
…ython#566

Return the loades and exported records from the admin functions

Export records in functional tests if an "export" directory exists

I spent a good amount of time trying to integrate this into the testing
suite more cleanrly (not in the _initialize_database function) but came
up with no solution that didn't require a large refator of the testing
code.
@kwilcox kwilcox changed the title WIP: Support indexing the ISO files from the IOOS Catalog Support indexing the ISO files from the IOOS Catalog Aug 27, 2018
@kwilcox
Copy link
Member Author

kwilcox commented Aug 27, 2018

@tomkralidis ready for review

msg = 'Cannot commit to repository'
LOGGER.exception(msg)
raise RuntimeError(msg)
raise
Copy link
Member

Choose a reason for hiding this comment

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

Will (only) raise emit msg?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, this just re-raises the same exception that was caught in the except block. This was used so I could catch DB errors in the admin.load_records function and return a specific error rather than a generic message/exception.

@tomkralidis tomkralidis merged commit f82fa0f into geopython:master Aug 27, 2018
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.

2 participants