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

Datajson Validator Route #122

Merged
merged 34 commits into from
Oct 7, 2022
Merged

Datajson Validator Route #122

merged 34 commits into from
Oct 7, 2022

Conversation

nickumia-reisys
Copy link

@nickumia-reisys nickumia-reisys commented Oct 4, 2022

Related to

Changes:

  • Fix make up by installing ckanext properly and dynamically adding files as necessary
  • Fix requests post data retrieval
  • Change default data.json display link from http://hub.healthdata.gov/data.json (which doesn't exist anymore to https://open.gsa.gov/data.json
  • Purge do_validation legacy code and all references to it
  • Make the jsonschema pod schema validation the default
  • Remove post-validation from data.json generation
  • Create new plugin datajson_validator which points to DataJsonValidatorPlugin
  • Rename route from pod/validator to dcat-us/validator and move to DataJsonValidatorPlugin
  • Separate templates for datajson and datajson_validator
    • templates_datajson are for templates related to datajson
    • templates_validator re for templates related to datajson_validator
  • Add tests for new DataJsonValidatorPlugin
  • Add example files to test errors in DataJsonValidatorPlugin

References:

Make sure app is setup to load at https://ckan:5000
Might be a py2 to py3 thing or just deprecated over the years.  Also, update the default data.json example
still has errors though
@github-actions
Copy link

github-actions bot commented Oct 4, 2022

✅ Result of Pytest Coverage

---------- coverage: platform linux, python 3.8.10-final-0 -----------

Name Stmts Miss Cover
ckanext/datajson/init.py 6 6 0%
ckanext/datajson/blueprint.py 227 47 79%
ckanext/datajson/datajson.py 436 113 74%
ckanext/datajson/datajsonvalidator.py 71 71 0%
ckanext/datajson/exceptions.py 2 0 100%
ckanext/datajson/harvester_base.py 3 0 100%
ckanext/datajson/harvester_cmsdatanavigator.py 63 63 0%
ckanext/datajson/harvester_datajson.py 49 13 73%
ckanext/datajson/helpers.py 111 43 61%
ckanext/datajson/package2pod.py 302 164 46%
ckanext/datajson/parse_datajson.py 105 32 70%
ckanext/datajson/plugin.py 34 1 97%
TOTAL 1409 553 61%
Required test coverage of
================== 34 passed, 3941

@nickumia-reisys nickumia-reisys marked this pull request as ready for review October 5, 2022 15:21
@nickumia-reisys nickumia-reisys marked this pull request as draft October 5, 2022 15:22
The old way of validating that there was a data.json link had flaws.  Make sure that the url parameter exists in post data, before doing anything else
It'll be the branch until it's merged into main
Per discussion with @FuhuXia, this old way of checking each field individually is undesirable.  There is the new way of pod schema validation courtesy of 'jsonschema' library.  All old references will be replaced in a coming commit.  The 'jsonschema' way already exists, it's just about tying everything together nicely
Call the jsonshema validator code and pass errors to the template; still WIP with print statements
Update the existing test to match the new output
it wasn't missing the right things
add more problems to tests
Handle all cases with a single method
- Prettify the --> to ➡
- Update existing tests for minor styling changes
- Create new Plugin Class 'DataJsonValidatorPlugin'
- Separate templates for each plugin
- Move /pod/validate route to new pluging
- Add plugin to setup.py
- Add plugin to list of plugins that get loaded on startup
If datajson_validator is enabled, the route is available.  If it is disabled, the route is not available
/pod/validate to /dcat-us/validator
@nickumia-reisys nickumia-reisys marked this pull request as ready for review October 6, 2022 20:02
@nickumia-reisys nickumia-reisys requested a review from a team October 6, 2022 20:03
@nickumia-reisys
Copy link
Author

I don't know if there's anything else to do. But if anyone sees anything, just let me know and I can add it to this PR.

Copy link

@btylerburton btylerburton left a comment

Choose a reason for hiding this comment

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

Looks great! Super clear to follow, and easy to continue to build on! Nice.

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.

3 participants