Skip to content

Conversation

kfindeisen
Copy link
Member

@kfindeisen kfindeisen commented Nov 27, 2023

This PR defines two new exceptions that let Prompt Processing specify whether a run should be retried, and adds exception handlers that understand them. The handlers are quite clumsy (in part because the desired behavior requires external I/O), so any suggestions for streamlining them would be most welcome.

This PR also cleans up our export implementation to prevent the confusing "exporting 0 datasets" messages that appeared previously.

@kfindeisen kfindeisen force-pushed the tickets/DM-34141 branch 2 times, most recently from 7dfb148 to 59d3d8e Compare November 27, 2023 21:16
Originally, export_outputs would only be called if the caller knew
there were datasets to export, so not finding any files implied the
data ID was wrong. Now that export_outputs may be called when the
pipeline failed at the beginning, this gives false positives. The
original message remains as a warning so that humans can check if it
came up unexpectedly.
These exceptions are intended as wrappers, to keep activator from
needing to know details of what exceptions the Middleware/pipeline code
can raise.
@kfindeisen kfindeisen force-pushed the tickets/DM-34141 branch 3 times, most recently from e13db7e to f8db1d9 Compare November 30, 2023 01:21
@kfindeisen kfindeisen marked this pull request as ready for review November 30, 2023 01:26
Copy link
Collaborator

@dspeck1 dspeck1 left a comment

Choose a reason for hiding this comment

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

Looks good. Like the text in the mock side effects!

These handlers determine whether the error is recoverable, and ensure
that any partial outputs are correctly synced to the central repo.
Exceptions raised outside of pipeline execution are still the
responsibility of the generic Flask handler.
This ensures that failures that have made permanent changes will never
be retried.
This covers states that have modified any of the APDB, the alert
stream, or the central repo.
Now that _export_subset uses Butler.transfer_from, it emits a log even
if there's nothing to export. This leads to a confusing log stream.
_export_subset can handle multiple input collections, so just handle
them all at once.
@kfindeisen kfindeisen merged commit 0cf37d7 into main Nov 30, 2023
@kfindeisen kfindeisen deleted the tickets/DM-34141 branch November 30, 2023 18:26
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