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 foreign key validation #86

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

ChasNelson1990
Copy link

@ChasNelson1990 ChasNelson1990 commented May 24, 2023

Description

closes #84

Basically, I have rewritten _validate_table to use the frictionless Package object, instead of the Resource object.

This allows us to then pass in multiple resources and do the following:

  • if "foreignKeys" is in the schema we collect together `schema["foreignKeys"][i]["reference"]["resource"]
  • we go through and check if the referenced resource is a url (path), json object (data) or (the fall back) is a resource name that can be found within this ckan dataset; if none are true then we return a ValidationError
  • these can be added to our Package object for frictionless framework to utilise during validation

Testing

TODO - I am working on these.

Documentation

I have updated the documentation to explain how to use foreign keys. It's fairly brief at the moment but probably in line with other similar parts.

@ChasNelson1990
Copy link
Author

I think the lint failure is kinda incorrect - the lint Action is using Python 3.7.16 (which, as flake8 is saying, did not have the walrus operator); however, the minimum Python version for ckan with Python 3 is 3.8 (as documented here: https://docs.ckan.org/en/2.10/maintaining/installing/install-from-source.html) and Python 3.8 introduced the walrus operator.

So I have pushed a small commit to this branch that brings the lint GitHub Action in line with the CKAN-chosen python version 3.8. I have tested locally using act (using the nektos/act-environments-ubuntu:18.04 image) as I don't have the permissions to dispatch on Github.

@ChasNelson1990
Copy link
Author

@amercader - I appreciate you haven't had chance to review this yet but I was wondering if you could approve the workflow so that I can check my lint action change is fully working? Thanks!

@ChasNelson1990 ChasNelson1990 force-pushed the foreign-key-validation branch 13 times, most recently from 169a8fc to 7b517b9 Compare July 21, 2023 16:45
@ChasNelson1990 ChasNelson1990 force-pushed the foreign-key-validation branch 3 times, most recently from 1645701 to 2ffa7e5 Compare July 21, 2023 18:13
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.

Support foreign keys property
2 participants