Skip to content

Commit

Permalink
Split IgnorableVisit and InvalidVisitError.
Browse files Browse the repository at this point in the history
We may want to reject nextVisit messages either because they specify
something we can't process (e.g., dome images for a differencing
pipeline) or because we specifically configured the service to ignore
them. The former is human error, the latter an error only in the formal
sense. Representing these cases with different exceptions lets them be
handled differently.
  • Loading branch information
kfindeisen committed Nov 5, 2024
1 parent dc7d981 commit 0043c9d
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 3 deletions.
13 changes: 11 additions & 2 deletions python/activator/activator.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@
import flask

from .config import PipelinesConfig
from .exception import GracefulShutdownInterrupt, InvalidVisitError, NonRetriableError, RetriableError
from .exception import GracefulShutdownInterrupt, IgnorableVisit, InvalidVisitError, \
NonRetriableError, RetriableError
from .logger import setup_usdf_logger
from .middleware_interface import get_central_butler, \
make_local_repo, make_local_cache, MiddlewareInterface
Expand Down Expand Up @@ -160,6 +161,7 @@ def create_app():

app = flask.Flask(__name__)
app.add_url_rule("/next-visit", view_func=next_visit_handler, methods=["POST"])
app.register_error_handler(IgnorableVisit, skip_visit)
app.register_error_handler(InvalidVisitError, invalid_visit)
app.register_error_handler(RetriableError, request_retry)
app.register_error_handler(NonRetriableError, forbid_retry)
Expand Down Expand Up @@ -385,6 +387,8 @@ def process_visit(expected_visit: FannedOutVisit):
`~activator.exception.NonRetriableError` or
`~activator.exception.RetriableError`, depending on the program state
at the time.
activator.exception.IgnorableVisit
Raised if the service is configured to not process ``expected_visit``.
activator.exception.InvalidVisitError
Raised if ``expected_visit`` is not processable.
activator.exception.NonRetriableError
Expand All @@ -409,7 +413,7 @@ def process_visit(expected_visit: FannedOutVisit):
assert expected_visit.instrument == instrument_name, \
f"Expected {instrument_name}, received {expected_visit.instrument}."
if not main_pipelines.get_pipeline_files(expected_visit):
raise InvalidVisitError(f"No pipeline configured for {expected_visit}.")
raise IgnorableVisit(f"No pipeline configured for {expected_visit}.")

log_factory = logging.getLogRecordFactory()
with log_factory.add_context(group=expected_visit.groupId,
Expand Down Expand Up @@ -528,6 +532,11 @@ def invalid_visit(e: InvalidVisitError) -> tuple[str, int]:
return f"Cannot process visit: {e}.", 422


def skip_visit(e: IgnorableVisit) -> tuple[str, int]:
_log.info("Skipping visit: %s", e)
return f"Skipping visit without processing: {e}.", 422


def request_retry(e: RetriableError):
error = e.nested if e.nested else e
_log.error("Processing failed but can be retried: ", exc_info=error)
Expand Down
15 changes: 14 additions & 1 deletion python/activator/exception.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
# along with this program. If not, see <https://www.gnu.org/licenses/>.


__all__ = ["NonRetriableError", "RetriableError", "GracefulShutdownInterrupt", "InvalidVisitError"]
__all__ = ["NonRetriableError", "RetriableError", "GracefulShutdownInterrupt",
"InvalidVisitError", "IgnorableVisit",
]


class NonRetriableError(Exception):
Expand Down Expand Up @@ -94,3 +96,14 @@ class InvalidVisitError(ValueError):
activator.visit.SummitVisit
activator.visit.FannedOutVisit
"""


class IgnorableVisit(ValueError):
"""An exception raised if a visit object has fields that are valid,
but that is configured to not be processed.
See Also
--------
activator.visit.SummitVisit
activator.visit.FannedOutVisit
"""

0 comments on commit 0043c9d

Please sign in to comment.