Implement a new Interface function for datastore_before_update#275
Conversation
| ''' | ||
| fields_match = _fields_match(fields, existing_fields, logger) | ||
| if fields_match == FieldMatch.EXACT_MATCH: | ||
| _notify_datastore_before_update(resource_id, existing_info, fields) |
There was a problem hiding this comment.
Line 33 uses kwargs, this uses args. Should be consistent.
ThrawnCA
left a comment
ThrawnCA
left a comment
There was a problem hiding this comment.
This looks good, thanks for adding it!
Just need to make the calls to the new function use a consistent argument style.
| mimetype="text/csv", | ||
| logger=logger, | ||
| ) | ||
|
|
|
|
||
| .. warning:: | ||
|
|
||
| The ``existing_fields`` and ``new_headers`` lists are the |
There was a problem hiding this comment.
I don't think the existing_fields object is the same after removing _id.
|
IIUC this is only useful for logging, but it looks fine to me. |
Theoretically a plugin could do things like overriding the data dictionary for specific fields when a resource is first created, instead of updating it later. Or eg allowing numeric fields while preventing any field from being treated as a timestamp. |
|
Hi @avdata99 , Overall the logic you wish to add looks good and should also allow other plugins to 'block' table changes if the wrong file is uploaded with column's changed, i.e. structured data used in api/rest calls or join's and should not be altered. Do ensure your new unit tests to also be passing. (It may be best to use a different branch for quick iterations and then squash the commits then merge them onto this branch for cleaner history). https://github.com/avdata99/ckanext-xloader/actions/runs/24799367527/attempts/1#summary-72577605178 (4 tests failing) Also, @wardi it looks like we will all need to put some time into updating xloader as we now have 44 baseline tests having failed against latest ckan/master branch. :'( |
|
Good catch @duttonw. Tests were updated here 94d7b68 Now they're green https://github.com/avdata99/ckanext-xloader/actions/runs/24833308497 |
4 participants
Adding the
datastore_before_updatehook toIXloaderI need to hook into the datastore updates to detect field changes
This PR adds a new optional method to the IXloader interface so plugins can observe schema changes before XLoader modifies the DataStore table.
The
datastore_before_updatecall includes:existing_fields: list of current DataStore field dicts (each has id, type, and optionally info), or None if the table does not yet exist. The internal
_idcolumn is stripped.new_headers: list of field dicts about to be written.
tests added. Ran here because of this repo permission