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

    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

            Activity

            Hide
            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
            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
            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
            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
            krzys Krzysztof Findeisen added a comment -

            Yes.

            Show
            krzys Krzysztof Findeisen added a comment - Yes.
            Hide
            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:

                  Summary Panel