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

Generalize job metadata code

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: verify
    • Labels:
      None
    • Story Points:
      2
    • Epic Link:
    • Sprint:
      AP S19-2
    • Team:
      Alert Production

      Description

      The SQuaSH-requested metadata are currently hardcoded into MetricsControllerTask. Generalize this system by delegating the work to a SQuaSH-specific subtask, as described in DMTN-098.

        Attachments

          Issue Links

            Activity

            Hide
            krzys Krzysztof Findeisen added a comment -

            Hi Gabor Kovacs, would you be willing to review this 200-line refactoring? Thanks!

            Show
            krzys Krzysztof Findeisen added a comment - Hi Gabor Kovacs , would you be willing to review this 200-line refactoring? Thanks!
            Hide
            gkovacs Gabor Kovacs added a comment - - edited

            Running test_squashMetadataTask.py as a standalone script falis and then complains about leaking objects, please see attachment. This is not produced as part of a pytest run.

             

            The failure seems to be fixed by changing "import unittest" to "import unittest.mock".

             

            The memory leaking in case of an exit with Python exceptions suggests a lower-level problem at the C++ wrapping.

             

            Otherwise, the changes look all right.

            Show
            gkovacs Gabor Kovacs added a comment - - edited Running test_squashMetadataTask.py as a standalone script falis and then complains about leaking objects, please see attachment. This is not produced as part of a pytest run.   The failure seems to be fixed by changing "import unittest" to "import unittest.mock".   The memory leaking in case of an exit with Python exceptions suggests a lower-level problem at the C++ wrapping.   Otherwise, the changes look all right.
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            Actually, it suggests a problem with the memory leak tests, which, among other flaws, do not distinguish between unreachable objects and objects that have been "stashed" unexpectedly. All it takes is a single reference on the Python side (including references held by the unit test framework itself), and your object leaks. See RFC-280 for more details.

            Though I'll admit I'm curious why these tests need to create a PropertySet in the first place.

            Show
            krzys Krzysztof Findeisen added a comment - - edited Actually, it suggests a problem with the memory leak tests, which, among other flaws, do not distinguish between unreachable objects and objects that have been "stashed" unexpectedly. All it takes is a single reference on the Python side (including references held by the unit test framework itself), and your object leaks. See RFC-280 for more details. Though I'll admit I'm curious why these tests need to create a PropertySet in the first place.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Thanks for the review!

            Show
            krzys Krzysztof Findeisen added a comment - Thanks for the review!

              People

              • Assignee:
                krzys Krzysztof Findeisen
                Reporter:
                krzys Krzysztof Findeisen
                Reviewers:
                Gabor Kovacs
                Watchers:
                Gabor Kovacs, Krzysztof Findeisen
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel