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

Resolve identifier URIs to MarkLogic URIs #794

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

dragon-dxw
Copy link
Collaborator

@dragon-dxw dragon-dxw commented Dec 6, 2024

Summary of changes

Functions to take a PUI url matching an identifier's URI and return the MarkLogic URI.
https://national-archives.atlassian.net/browse/FCL-496

Checklist

  • I have created/updated method docstrings (if necessary)
  • I have considered if this is a breaking change
  • I have updated the changelog (if necessary)

@dragon-dxw dragon-dxw force-pushed the FCL-496-8-resolve-identifier-urls branch 2 times, most recently from 03189bc to a64c439 Compare December 9, 2024 11:33
@@ -0,0 +1,31 @@
import json
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm a bit conflicted about where this file lives -- I think this is better than being in models.identifiers. Maybe it should be in models.identifier_resolution ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree this isn't a data model - current location seems fine to me.

@dragon-dxw dragon-dxw force-pushed the FCL-496-8-resolve-identifier-urls branch from a64c439 to 9163483 Compare December 9, 2024 12:39
@dragon-dxw dragon-dxw marked this pull request as ready for review December 9, 2024 15:52
Copy link
Collaborator

@jacksonj04 jacksonj04 left a comment

Choose a reason for hiding this comment

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

Needs a touch more in docstrings and I think we should flip a default to the 'safe' choice, otherwise LGTM!

@@ -0,0 +1,31 @@
import json
Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree this isn't a data model - current location seems fine to me.


@staticmethod
def from_marklogic_output(raw_row: str) -> "IdentifierResolution":
row = json.loads(raw_row)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would be a candidate for refactoring in future if we do much more with TDE; possibly one to touch on when we improve how EUI reports on things in various pending states, or when we do exception reporting.

src/caselawclient/identifier_resolution.py Show resolved Hide resolved

declare namespace xdmp="http://marklogic.com/xdmp";
declare variable $identifier_uri as xs:string external;
declare variable $published_only as xs:int? external := 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it safer for this to default to being true, so we have to explicitly request that it include unpublished stuff?

@dragon-dxw dragon-dxw force-pushed the FCL-496-8-resolve-identifier-urls branch from fd63e5c to 5a50e75 Compare December 9, 2024 16:13
@dragon-dxw dragon-dxw enabled auto-merge December 9, 2024 16:13
@dragon-dxw dragon-dxw added this pull request to the merge queue Dec 9, 2024
Merged via the queue into main with commit 8eb73c6 Dec 9, 2024
10 of 12 checks passed
@dragon-dxw dragon-dxw deleted the FCL-496-8-resolve-identifier-urls branch December 9, 2024 16:16
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