Skip to content

Preservica fetcher and mapper - #1333

Open
barbarahui wants to merge 10 commits into
mainfrom
preservica
Open

Preservica fetcher and mapper#1333
barbarahui wants to merge 10 commits into
mainfrom
preservica

Conversation

@barbarahui

@barbarahui barbarahui commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

This PR:

  1. adds a new preservica fetcher
  2. adds a new preservica mapper
  3. reads preservica access credentials out of the airflow environment variables instead of from registry

I added the stateLocatedIn mapping since that existed in the legacy mapper, even though it looks like we didn't implement the related enrichment in Rikolti. I noticed that we've ported over this mapping for a few other collections too.

@barbarahui
barbarahui force-pushed the preservica branch 3 times, most recently from 084786b to 0fe2c4e Compare July 9, 2026 21:33
@barbarahui barbarahui changed the title Beginnings of preservica fetcher Preservica fetcher and mapper Jul 9, 2026
@barbarahui
barbarahui force-pushed the preservica branch 3 times, most recently from ba8bef6 to 5b039b4 Compare July 10, 2026 23:23
@barbarahui barbarahui linked an issue Jul 13, 2026 that may be closed by this pull request
@barbarahui
barbarahui force-pushed the preservica branch 2 times, most recently from fc6380e to 84137dc Compare July 14, 2026 18:35
@barbarahui barbarahui linked an issue Jul 15, 2026 that may be closed by this pull request
@barbarahui
barbarahui marked this pull request as ready for review July 15, 2026 16:32
@barbarahui
barbarahui requested a review from amywieliczka as a code owner July 15, 2026 16:32
@barbarahui
barbarahui marked this pull request as draft July 15, 2026 16:34
@barbarahui
barbarahui marked this pull request as ready for review July 15, 2026 22:20
@barbarahui
barbarahui force-pushed the preservica branch 3 times, most recently from 25e1664 to 92e9e57 Compare July 15, 2026 23:18
@barbarahui
barbarahui requested a review from bibliotechy July 24, 2026 17:13
@barbarahui barbarahui mentioned this pull request Jul 24, 2026

@bibliotechy bibliotechy left a comment

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.

Looks good, one comment. Thanks for fighting with a weird system :)


fetched_page = http_resp.json()
self.num_fetched += len(fetched_page.get("value", {}).get("metadata", []))
if self.num_fetched >= fetched_page.get("value", {}).get("totalHits"):

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.

There is a risk that if totalHits is not in the dictionary then this will error as it will try to compare an int to None. Might be worth explicitly try / except for that condition just to avoid a future debug dive on what might jsut be a transitory issue.

bibliotechy
bibliotechy previously approved these changes Jul 24, 2026
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.

PreservicaAPIMapper(Mapper) Fetcher: PRA

4 participants