merge: bring CLARIN branch (main) into dtq - #17
Merged
Conversation
missing functions for updating resource policies
removed metadata
added License and Label models
Sync dtq with main
… fetch bitstream, fetch bundle, get user by email, get http status, logout
…lowance-endpoints
test for access to user-metadata and user-allowance endpoints
…bmitter UFAL/Run swordv2 tests as submitter
get_clarinlruallowances_by_bitstream_and_user() requested the singular
collection /core/clarinlruallowance/search/byBitstreamAndUser. DSpace 9
routes REST repositories by their declared plural name, so that path
returns:
404 {"message": "The repository type core.clarinlruallowance was not found"}
The caller treats the empty result as "no allowance exists" and reports
NO_ALLOWANCE, which surfaced in dspace-rest-test as 2 failures and 2
errors in test_endpoints against CLARIN DSpace 9.3 (dtq-dev-9-base) --
even though the allowance was present all along.
The plural form is backward compatible: verified test_endpoints passes
8/8 against both DSpace 9.3 (dev-6) and DSpace 7.6.5 (dev-5), so no
version switch is needed.
Note the other two CLARIN URLs in this client are already correct and
are left untouched: /core/clarinlruallowances (already plural) and
/core/clarinusermetadata/manage (an explicit @RequestMapping that takes
precedence over repository routing -- returns 403 on both 7.6.5 and 9.3).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removed note about collection name pluralization for DSpace.
fix: use plural collection name for clarinlruallowances search
origin/main and origin/dtq had diverged since 5724696 (2024-10-24) with no ancestry either way. main added +300/-0 lines and dtq +263/-71, both confined to dspace_rest_client/client.py and dspace_rest_client/models.py. main contributes the CLARIN/UFAL surface: License and Label models, clarinlruallowances get/create, submit-group creation, eperson-to-group membership, get_item_by_handle, get_items_from_collection, get_bundle_by_name, get_user_by_email, api_put_uri and the dict-based resource-policy helpers. dtq keeps its hardening: reauth on 401, request timeouts, proxy support, verify_response, _last_err, the ResourcePolicy model and 404-safe bundles. Conflict resolutions (3 hunks, 2 in client.py and 1 in models.py): - get_items: kept dtq's paginated get_items(self, page=0, size=20) and dropped main's no-arg variants. main still carried two duplicate `def get_items` definitions; the shadowing second one tested for 'collections' in _embedded while iterating _embedded['items']. The paginated signature is required by DSpace-ISstag-integration src/ingest/_dspace.py, which calls get_items(page=page, size=page_size). main's additive get_item_by_handle is kept alongside it. - remove_metadata: unified the two signatures into remove_metadata(self, dso, field, place=None). place=None removes every value of the field (dtq behaviour, PATCH /metadata/{field}); an explicit place removes a single value (main behaviour, PATCH /metadata/{field}/{place}). This keeps dtq's 2-arg call sites and dspace-import-clarin's remove_metadata(item, key, 0) working. main's module-level logging.error was replaced by _logger.error to match the rest of the file. - models.py: kept all three new classes - License (with to_dict), Label and ResourcePolicy (with as_dict and __repr__). The conflict was purely positional, both sides having appended at EOF. api_put_uri was added on main against the pre-hardening base, so it was harmonised with its sibling api_put: reset self._last_err on entry, pass proxies=self.proxies, and use _logger instead of module-level logging. Its public signature is unchanged - dspace-rest-test depends on it. dtq-dev is deliberately NOT merged. Its only unique commit, 145635a (2023-06-12, "copied code from dtq main"), is a snapshot of a different project: it relocates dspace_rest_client/{client,models}.py into support/dspace_interface/, deletes setup.py, example.py, example_gets.py, solr_example.py, publish.sh, CHANGELOG.md and MAINTAINING.md, and adds ~8.5k lines of import tooling, test fixtures, license icons and a localization CSV. Merging it would destroy the installable package. Both resource-policy APIs are retained side by side, since main's dict-based set (get_resource_policy, create_resource_policy, update_resource_policy_group) and dtq's model-based set (get_resourcepolicy, create_resourcepolicy) each have live consumers. Unification is left to a separate deprecation change. Verified: compileall clean, package imports, no duplicate method definitions, and the merged API surface is exactly the union of both branches (nothing from either side lost, main's duplicate get_items aside). The DSpace-ISstag-integration suite gives an identical result with and without this merge - 579 passed, 40 failed - where the 40 failures are pre-existing and caused by flask being absent from that environment, not by the library. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Merges CLARIN/UFAL “main” lane functionality into the dtq lane so downstream consumers of dtq (as a submodule) gain the CLARIN API surface while keeping dtq’s hardened client behavior and compatibility-sensitive method signatures.
Changes:
- Extended the client with
api_put_uri()plus multiple CLARIN/group/resource-policy helper methods. - Resolved API compatibility conflicts by keeping
get_items(page,size)and expandingremove_metadata(..., place=None)to a backwards-compatible superset. - Added CLARIN
License/Labelmodels while retaining theResourcePolicymodel.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
dspace_rest_client/client.py |
Adds api_put_uri() and multiple new endpoints (CLARIN allowances, submit-group membership, resource policy helpers), plus a compatible remove_metadata() signature. |
dspace_rest_client/models.py |
Adds License and Label models alongside the existing ResourcePolicy model. |
Suppressed comments (1)
dspace_rest_client/client.py:1401
get_clarinlruallowances_by_bitstream_and_user()has the same empty-list bug asget_clarinlruallowances()(returnsNonewhen the API returns[]). It also uses a broadexcept Exceptioninstead of handling request/JSON failure explicitly.
try:
response = self.api_get(url, params=params)
data = parse_json(response)
allowances = data.get('_embedded', {}).get('clarinlruallowances')
if allowances:
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1244
to
+1246
| url = f'{self.API_ENDPOINT}/eperson/groups/{group.uuid}/epersons' | ||
| eperson_uri = f'{self.API_ENDPOINT}/epersons/{eperson.uuid}' | ||
| r = self.api_post_uri(url, params=None, uri_list=eperson_uri) |
Comment on lines
+1381
to
+1385
| try: | ||
| response = self.api_get(url) | ||
| data = parse_json(response) | ||
| allowances = data.get('_embedded', {}).get('clarinlruallowances') | ||
| if allowances: |
Comment on lines
+1431
to
+1439
| url = f'{self.API_ENDPOINT}/eperson/epersons/search/byEmail' | ||
| params = {'email': email} | ||
| try: | ||
| response = self.api_get(url, params=params) | ||
| user_data = parse_json(response) | ||
| return User(user_data) | ||
| except Exception as e: | ||
| _logger.error(f"Error retrieving user by email {email}: {e}") | ||
| return None |
Comment on lines
+554
to
+556
| """ | ||
| Specific attributes and functions for licenses | ||
| """ |
Author
There was a problem hiding this comment.
possible docs fix also for main
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
dataquest-dev/dspace-rest-pythonhas three long-lived branches with no ancestry between any of them.dtqis the branch theDSpace-ISSTAG-Integrationfamily consumes as a submodule, so it needs the useful work from the other lanes.mainmainclarinlruallowances, submit-group, eperson-to-groupdbec1512026-08-11dtqf9ca9422026-08-17dtq-dev145635a2023-06-12Merge base of
mainanddtqis5724696("merged", 2024-10-24). Since thenmainadded +300/−0 lines anddtq+263/−71, both confined todspace_rest_client/client.pyanddspace_rest_client/models.py. No build files, deps or layout diverged.What this PR does
Merges
origin/mainintodtqviamerge/main-into-dtq.dtqgains main's CLARIN surface; nothing of dtq's hardening is lost.Gained from
main(13 new methods + 2 new models):get_clarinlruallowances,get_clarinlruallowances_by_bitstream_and_user,create_clarinlruallowances,create_submit_group,add_member,get_user_by_email,get_bundle_by_name,get_items_from_collection,get_item_by_handle,api_put_uri,get_resource_policy,create_resource_policy,update_resource_policy_group, plus theLicenseandLabelmodels.Kept from
dtq: reauth on 401, request timeouts, proxy support,verify_response,_last_err,ResourcePolicymodel,get_resourcepolicy/create_resourcepolicy,get_owningCollection, 404-safe bundles.Conflict resolutions
3 hunks — 2 in
client.py, 1 inmodels.py.1.
get_items— signature clash (breaking if resolved wrong)dtq:get_items(self, page=0, size=20)— sendspage/sizeparams.main:get_items(self)— andmainstill carried two duplicatedef get_itemsdefinitions. The shadowing second one tested for'collections'in_embeddedwhile iterating_embedded['items'].Resolved: kept dtq's paginated version, dropped both of main's.
DSpace-ISstag-integration/src/ingest/_dspace.pycallsget_items(page=page, size=page_size), so main's signature would break the STAG import loop.dspace-rest-testcallsget_items()with no args — satisfied by the defaults. main's additiveget_item_by_handleis kept alongside.2.
remove_metadata— signature clash (breaking if resolved wrong)dtq:remove_metadata(self, dso, field)→PATCH /metadata/{field}(removes all values).main:remove_metadata(self, dso, field, place)→PATCH /metadata/{field}/{place}(removes one value); also regressed to module-levellogging.error.Resolved to a compatible superset:
Both dtq's 2-arg call sites and
dspace-import-clarin'sremove_metadata(item, key, 0)keep working._loggerrestored.3.
models.py— positional conflictBoth sides appended at EOF. Resolved by keeping all three:
License(withto_dict),Label(withto_dict),ResourcePolicy(withas_dictand__repr__). Trailing newline added.One extra change beyond conflict resolution
api_put_uriwas written onmainagainst the pre-hardening base, so the merge would have imported it without any of dtq's conventions. Harmonised with its siblingapi_put: resetsself._last_erron entry, passesproxies=self.proxies, and uses_loggerinstead of module-levellogging. Public signature unchanged —dspace-rest-testuses it in 3 places.All of main's other new methods go through
self.api_get/self.api_post, so they inherit dtq's hardening for free.Why
dtq-devis NOT mergedIts only unique commit —
145635a(2023-06-12, "copied code from dtq main") — is a snapshot of a different project, the DSpace import/migration tool. It:dspace_rest_client/{client,models}.py→support/dspace_interface/setup.py,example.py,example_gets.py,solr_example.py,publish.sh,CHANGELOG.md,MAINTAINING.mdMerging it would destroy the installable package and contributes zero library value. It is 1 ahead / 64 behind
mainand an ancestor of nothing.Both resource-policy APIs retained
mainanddtqindependently grew policy APIs with different names and different return shapes, and both have live consumers (dspace-import-clarinuses main's dict-based set; all fiveDSpace-ISstag-integration*clones use dtq's model-based set). They occupy different regions ofclient.py, so git merged both cleanly. Unification is deliberately left to a separate deprecation change.Verification
python -m compileall -q dspace_rest_client— cleanDSpaceClient,Label,License,ResourcePolicy,Item,Bundle,Bitstream,Community,Collection) — OKclient.py(uniq -dempty); no duplicate class names inmodels.pyget_itemsget_items(self, page=0, size=20),remove_metadata(self, dso, field, place=None),api_put_uri(self, url, params, uri_list, retry=False)Label.to_dict(),License.to_dict()(including nestedclarinLicenseLabel),ResourcePolicy.as_dict()/__repr__()(including_embedded.groupextraction)DSpace-ISstag-integrationsuite: identical result with and without this merge — 579 passed, 40 failed. The 40 failures are pre-existing, all intests/test_app_security.py, caused byflaskbeing absent from that environment (ModuleNotFoundError: No module named 'flask'atwww-stats/web/be/app.py:37), not by the library.dspace-import-clarinanddspace-rest-testchecked statically against the merged signatures.No live-backend smoke test was run — worth doing against a dev DSpace before the submodule pins are bumped.
Follow-ups (not in this PR)
DSpace-ISstag-integration*clones declarebranch = dtq-devin.gitmoduleswhile pinning a commit ondtq.git submodule update --remotewould regress them to 2023 code. Should bebranch = dtq.e1c05a99(2026-02-23) to the post-mergedtqtip — also picks up51b65fe(timeout),db03111(robust check),f9ca942(404-safe bundles).dtq-dev— archive as a tag, then delete the branch.dspace-import-clarinto theResourcePolicymodel, then remove.added-post-with-text(2025-07-04) andfeature/upstream-pagination-and-new-features(2026-02-13, "upstream pagination, iterators, embeds, and new API methods"). Triage separately.the-library-code/dspace-rest-python(v0.1.20). Separate project.get_itemsyears ago.🤖 Generated with Claude Code