Skip to content

Resource View & DataStore Fields for Loading Objects#229

Open
JVickery-TBS wants to merge 10 commits into
ckan:masterfrom
JVickery-TBS:feature/resource-options
Open

Resource View & DataStore Fields for Loading Objects#229
JVickery-TBS wants to merge 10 commits into
ckan:masterfrom
JVickery-TBS:feature/resource-options

Conversation

@JVickery-TBS
Copy link
Copy Markdown
Contributor

Adds the ability to load the resource_views and datastore_fields from the CKANAPI dump options.

cc: @wardi

- Load resource views from ckanapi dump.
- Load resource datastore fields from ckanapi dump.
- Update the cli help text.
- Update the cli help text.
- Update the cli help text.
Comment thread ckanapi/cli/main.py Outdated
Comment thread ckanapi/cli/load.py
Comment thread ckanapi/cli/load.py Outdated
Comment thread ckanapi/cli/load.py Outdated
Comment thread ckanapi/cli/load.py Outdated
r = ckan.call_action(thing_create, args,
requests_kwargs=requests_kwargs)
except ValidationError as e:
reply(act, 'ValidationError', e.error_dict, stdout)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the validation error case I think we want to only catch the case where the table already exists and then skip applying the fields. We could return a value to include in the reply that indicates that datastore fields were not applied to some resources, but I don't think this should be treated as an error.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

cuz we are in a loop of the resources, how would we return which ones did not get created? just as a list of resource ids?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay I kinda made something for the extra output for datastore_fields and resource_views. So it would display what was created, updated, and skipped.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay I THINK I did it, so catch ValidationError. If there is no existing table, then raise the error and the worker process will scream for that line. If there is an existing table, I just append the resource id and string error to the "skipped" tables which outputs in the worker process log

- Update the cli help text.
- Update the cli help text.
@JVickery-TBS JVickery-TBS requested a review from wardi May 11, 2026 15:34
@JVickery-TBS
Copy link
Copy Markdown
Contributor Author

@wardi okay I believe I have achieved everything here now

Comment thread ckanapi/cli/load.py
reply(act, 'NotFound', obj)
else:
reply(act, None, r.get('name',r.get('id')))
log_obj = {}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about we switch to always returning a dict with id and name values and optionally these other things, instead of sometimes returning a string and sometimes a dict, depending on options passed

Comment thread ckanapi/cli/load.py
Comment on lines +374 to +376
requests_kwargs = None
if arguments['--insecure']:
requests_kwargs = {'verify': False}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This repeated pattern suggests we should add requests_kwargs to RemoteCKAN.__init__ so that we don't have to pass them on every call_action

Comment thread ckanapi/cli/load.py
# exceptions handled in load_things_worker
# raise normal exception for non-existing tables
raise e
skipped.append('%s: %s' % (rid, str(e)))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure the str(e) happens automatically as part of the string formatting

Suggested change
skipped.append('%s: %s' % (rid, str(e)))
skipped.append('%s: %s' % (rid, e))

@wardi
Copy link
Copy Markdown
Contributor

wardi commented May 15, 2026

Tests can be added by creating mocks for the actions. We should at least cover the happy path and exceptions from each action to make sure they're handled.

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