Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-13803

Bypass handling catches too many exceptions

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: daf_persistence
    • Labels:
      None
    • Team:
      Architecture

      Description

      Michael Wood-Vasey points out that the bypass handling in the Butler get method catches all exceptions to allow searching of other repositories in case a dataset doesn't exist.  This is excessive; RuntimeError exceptions should cause failures, and (I think) only NoResults exceptions should cause continued searching.

        Attachments

          Issue Links

          relates to

          Epic - Created by Jira Software - do not edit or delete. Issue type for a big user story that needs to be broken down. DM-14310 Fix catching of generic Exceptions

          • To Do

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            wmwood-vasey Michael Wood-Vasey added a comment - - edited

            Thank you. The current change looks good and would have immediately revealed the error. I attach two files showing the difference. The master.log ends in a generic No locations for get, while the second DM-13803.log ends directly with the failed key lookup in a template format string that was the root cause.

            Take this as a pre-emptive review. Presuming this passes Jenkins, this looks good to me.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - - edited Thank you. The current change looks good and would have immediately revealed the error. I attach two files showing the difference. The master.log ends in a generic No locations for get , while the second DM-13803.log ends directly with the failed key lookup in a template format string that was the root cause. Take this as a pre-emptive review. Presuming this passes Jenkins, this looks good to me.
            Hide
            Permalink
            ktl Kian-Tat Lim added a comment -

            A small, hopefully quick-to-review, change.  Passes Jenkins.

            Show
            ktl Kian-Tat Lim added a comment - A small, hopefully quick-to-review, change.  Passes Jenkins.
            Hide
            Permalink
            npease Nate Pease [X] (Inactive) added a comment -

            I raised a question about logging in the catch statement, otherwise it all looks fine and like a good improvement.

            Show
            npease Nate Pease [X] (Inactive) added a comment - I raised a question about logging in the catch statement, otherwise it all looks fine and like a good improvement.
            Hide
            Permalink
            ktl Kian-Tat Lim added a comment -

            Thanks for the log suggestion; implemented, CI-tested, and merged.

            Show
            ktl Kian-Tat Lim added a comment - Thanks for the log suggestion; implemented, CI-tested, and merged.

              People

              Assignee:
              ktl Kian-Tat Lim
              Reporter:
              ktl Kian-Tat Lim
              Reviewers:
              Nate Pease [X] (Inactive)
              Watchers:
              Kian-Tat Lim, Michael Wood-Vasey, Nate Pease [X] (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.