Skip to content

Conversation

@ghukill
Copy link
Contributor

@ghukill ghukill commented Dec 12, 2025

Purpose and background context

Why these changes are being introduced:

If there are any errors loading an exclusion list for a source, e.g. the file is not accessible or not parseable, we want the transformation process to fail completely without writes to the TIMDEX dataset.

As-is, we found that all records from the run will write to the dataset but with action="error". This is good when some records fails to understand what they were and why, but an exclusion list error will fail for all records making this not helpful.

How this addresses that need:

A new custom exception CriticalError is introduced that when raised will break the Transformer.__next__() iterator. In turn, this will bubble up to the CLI level and terminate the run with a non-zero process.

This provides a mechanism from within a Transformer's transformation logic to raise an exception that will break the Transformer.__next__ iteration.

Side effects:

  • Failure to load an exclusion list will terminate the run early and explicitly.
  • In the future, catastrophic errors that suggest all records in the run will fail should also raise this new, custom exception.

How can a reviewer manually see the effects of these changes?

1- Copy a fixture to use as an input file:

cp tests/fixtures/oai_dc/springshare/libguides/libguides_record_all_fields.xml output/libguides-2025-12-12-full-extracted-records-to-index.xml

2- Run transform with an exclusion list that doesn't exist:

pipenv run transform -s libguides -i output/libguides-2025-12-12-full-extracted-records-to-index.xml -o /tmp/dataset -e /tmp/does-not-exist.csv

3- Check exit code:

echo $?

Note the CLI process terminates with a non-zero exit code, and a clear reason why:

transmogrifier.exceptions.CriticalError: Could not load exclusion list: [Errno 2] No such file or directory: '/tmp/does-not-exist.csv'

This will stop the StepFunction at this stage and not move on to future steps.

Includes new or updated dependencies?

YES | NO

Changes expectations for external applications?

NO

What are the relevant tickets?

Code review

  • Code review best practices are documented here and you are encouraged to have a constructive dialogue with your reviewers about their preferences and expectations.

Why these changes are being introduced:

If there are any errors loading an exclusion list for a source, e.g.
the file is not accessible or not parseable, we want the transformation process
to fail completely without writes to the TIMDEX dataset.

As-is, we found that all records from the run will write to the dataset but with
action="error".  This is good when *some* records fails to understand what they
were and why, but an exclusion list error will fail for *all* records making
this not helpful.

How this addresses that need:

A new custom exception `CriticalError` is introduced that when raised
will break the Transformer.__next__() iterator.  In turn, this will
bubble up to the CLI level and terminate the run with a non-zero
process.

This provides a mechanism from within a Transformer's transformation
logic to raise an exception that will break the Transformer.__next__
iteration.

Side effects of this change:
* Failure to load an exclusion list will terminate the run early
and explicitly.
* In the future, catastrophic errors that suggest all records in
the run will fail should also raise this new, custom exception.

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/USE-278
@ghukill ghukill marked this pull request as ready for review December 12, 2025 20:59
@ghukill ghukill requested a review from a team December 12, 2025 20:59
@ehanson8 ehanson8 self-assigned this Dec 15, 2025
Copy link
Contributor

@ehanson8 ehanson8 left a comment

Choose a reason for hiding this comment

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

Works as expected and a smart update!

@ghukill ghukill merged commit c1803cb into main Dec 15, 2025
3 checks passed
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.

3 participants