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

Freezing a config locks the registry(ies) of other instances of that config

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: pex_config
    • Labels:
      None
    • Story Points:
      8
    • Epic Link:
    • Sprint:
      DRP S20-6 (May)
    • Team:
      Data Release Production
    • Urgent?:
      No

      Description

      Config field instances that correspond to a RegistryField have an internal member, _field, pointing back to the original field. _field points to the same object for all config instances corresponding to the same config. This causes a rare bug where calling ConfigChoiceField.freeze locks the registry for all config instances using that RegistryField, including instances that did not exist at the time of the call. See my post on #dm for a specific example.

      Please change the code so that independently created config instances no longer affect each other. This issue appears to be a side effect of the fix for DM-17757, and may be difficult to fix without introducing still other side effects.

        Attachments

          Issue Links

          relates to

          Bug - A problem which impairs or prevents the functions of the product. DM-17757 Key error when trying to use pipetask to output pipeline/quantum graph and then read it.

          • Done

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-24259 Create “stub“ Gen2 HSC dataset for CI testing

          • Done

          Improvement - An improvement or enhancement to an existing feature or task. DM-24436 Use obs and camera overrides in make_apdb.py

          • Won't Fix

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            krzys Krzysztof Findeisen added a comment -

            I wonder if the solution isn't to, on freezing the config, copy the RegistryField/ConfigChoiceField instead of calling its freeze() method. You'd still have the benefits of frozen registries, but they'd be decoupled from the original, publicly visible RegistryField.

            Show
            krzys Krzysztof Findeisen added a comment - I wonder if the solution isn't to, on freezing the config, copy the RegistryField / ConfigChoiceField instead of calling its freeze() method. You'd still have the benefits of frozen registries, but they'd be decoupled from the original, publicly visible RegistryField .
            Hide
            Permalink
            nlust Nate Lust added a comment -

            I pushed a pex config branch, can you see if that works for you? If so I will clean it up a bit.

            Show
            nlust Nate Lust added a comment - I pushed a pex config branch, can you see if that works for you? If so I will clean it up a bit.
            Hide
            Permalink
            krzys Krzysztof Findeisen added a comment -

            Yes, ap_verify runs and produces the same output.

            Show
            krzys Krzysztof Findeisen added a comment - Yes, ap_verify runs and produces the same output.
            Hide
            Permalink
            nlust Nate Lust added a comment -

            I just want to verify the sign of your comment. This does fix your initial issue, without your work around on ap_pipe?

            Show
            nlust Nate Lust added a comment - I just want to verify the sign of your comment. This does fix your initial issue, without your work around on ap_pipe?
            Hide
            Permalink
            krzys Krzysztof Findeisen added a comment -

            Yes.

            Show
            krzys Krzysztof Findeisen added a comment - Yes.
            Hide
            Permalink
            jbosch Jim Bosch added a comment -

            A few style comments on the PR.

            Show
            jbosch Jim Bosch added a comment - A few style comments on the PR.

              People

              Assignee:
              nlust Nate Lust
              Reporter:
              krzys Krzysztof Findeisen
              Reviewers:
              Jim Bosch
              Watchers:
              Jim Bosch, Krzysztof Findeisen, Nate Lust
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.