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

In plugin.py, there is an fix of resource format key error #209

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

Nisha1293
Copy link
Contributor

This is the fix of resource format key error

This is the fix of resource format key error
@Nisha1293
Copy link
Contributor Author

Hi @duttonw
I have raised one PR. Could you please review it.

@duttonw
Copy link
Collaborator

duttonw commented Feb 28, 2024

Hi @Nisha1293,

Can you enable github actions on your repo and retrigger the build so i can review the tests etc.
https://github.com/Nisha1293/ckanext-xloader/actions

regards,

@duttonw

@ThrawnCA
Copy link
Collaborator

I don't understand what situation would fail to have a format.

@duttonw
Copy link
Collaborator

duttonw commented Feb 29, 2024

I think its in relation to not having 'format' in the dict. Some ckan admin's may have decided to drop that field on their servers.

@ThrawnCA
Copy link
Collaborator

Some ckan admin's may have decided to drop that field on their servers.

And still run XLoader, which relies on the format field to know whether it should try to parse a resource? That seems very odd indeed.

@duttonw
Copy link
Collaborator

duttonw commented Jul 2, 2024

@ThrawnCA, I don't see any issue in merging this since its just wrapped it in a null safe context.

Your thoughts?

@ThrawnCA
Copy link
Collaborator

ThrawnCA commented Jul 2, 2024

Your thoughts?

I suppose it's harmless, it just seems very odd for anyone to need this null-check.

@duttonw duttonw merged commit 3a865ac into ckan:master Jul 2, 2024
3 checks passed
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