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

IPipeValidation Interface #97

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

Conversation

JVickery-TBS
Copy link

feat(dev): IPipeValidation interface;

  • Created new IPipeValidation interface which sends dictized validation reports to other plugins.

Based implementation off of the Archiver plugin's IPipe https://github.com/ckan/ckanext-archiver/blob/master/ckanext/archiver/interfaces.py#L9

This would be very useful to plugins like Xloader and Datapusher to check a resource's validation and continue any processes to get the data into the DataStore.

- Created new `IPipeValidation` interface which sends dictized validation reports to other plugins.
@duttonw
Copy link

duttonw commented May 27, 2024

Hi @amercader,

Are you able to look over this and see if we can get this merged in and a version cut?

If you are too busy, would you like to provide @ThrawnCA or myself maintaining access to keep this plugin fresh?

This work that @JVickery-TBS is doing is very useful in allowing the xloader/datapusher to postpone its operations until we have a green from the validation sub-system that the file won't corrupt the previously loaded datastore.

kind regards,

@duttonw

@wardi
Copy link

wardi commented Jun 28, 2024

@amercader why don't we fork this repo to the ckan github org and give @ThrawnCA and @duttonw maintainer permissions there?

@amercader
Copy link
Member

@duttonw @ThrawnCA @JVickery-TBS @wardi

I think it's a great idea, as long as the frictionless folks are happy with it. @pdelboca @roll what do you think?

@pdelboca
Copy link
Member

pdelboca commented Jul 1, 2024

@amercader @duttonw @ThrawnCA @wardi @roll @JVickery-TBS

I think moving it to CKAN github org is a great idea. This extension has a lot of potential, I think it will integrate really well with the latest Table Designer feature of CKAN 2.11.

+1

@wardi
Copy link

wardi commented Jul 2, 2024

Sounds great, moving the repo is even better than forking it. I don't have permission to move a repo from frictionlessdata but maybe @pdelboca or @amercader do?

@amercader
Copy link
Member

Repo transferred.

@duttonw @ThrawnCA you should have write access to this repo (or have a pending invite to join)

@duttonw
Copy link

duttonw commented Jul 11, 2024

Repo transferred.

@duttonw @ThrawnCA you should have write access to this repo (or have a pending invite to join)

Thanks, invite accepted.

@ThrawnCA
Copy link
Contributor

ThrawnCA you should have write access to this repo (or have a pending invite to join)

I have write access, yes.

@duttonw
Copy link

duttonw commented Jul 23, 2024

Hi @JVickery-TBS ,

I've got a pickle of a problem on this now that i've been provided repo rights.
The master branch is currently not passing 2.9 tests (unsure when it last worked and it was not stuck on docker container changes).

Am a little sad that 2.10 is not in the matrix, I do see that it was uplifted to 2.10 via this PR https://github.com/ckan/ckanext-validation/pull/66/files so :/

While this is sorted, can you open a PR against https://github.com/qld-gov-au/ckanext-validation and have this interface added, while we sort out how to get these diverged tree's amalgamated (maybe with some feature flags).

@JVickery-TBS
Copy link
Author

@duttonw Yeah I started upgrading to 2.10 this summer and also found out that it does not support it yet. I started adding in some super basic support: https://github.com/open-data/ckanext-validation/tree/canada-v2.10

But your fork looks really good. Let me know if you are planning to merge a lot of your fork into here ckan/ckanext-validation, and I will merge yours into our fork too.

Will work on putting this IPipeValidation into your fork though.

@ThrawnCA
Copy link
Contributor

ThrawnCA commented Aug 8, 2024

@JVickery-TBS You need to update the Solr test container name from the obsolete "ckan-solr-dev" to just "ckan-solr".

- Update solr image. in github test workflow.
- Fix the auth test cases.
@duttonw
Copy link

duttonw commented Aug 19, 2024

Hi @JVickery-TBS ,

Before I Merge this, can you also update the README.md with what you have changed.

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.

6 participants