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

PipelineTask single-config override does not parse booleans correctly.

    Details

    • Story Points:
      1
    • Sprint:
      BG3_F18_11
    • Team:
      Data Access and Database

      Description

      Options like -c label.option=True don't seem to work. If needed, I think Yusra AlSayyad has a more complete how-to-reproduce.

        Attachments

          Activity

          Hide
          salnikov Andy Salnikov added a comment -

          We do need better support for parsing values of the options on the command line (Nate has already started improving support for lists). One way to do it is just eval() the value (or safer ast.literal_eval()) but that is not super-convenient in all cases as one would need to quote regular strings. I think better option would be to check field dtype and only do eval if dtype is not str. I'll try to implement that to see if that can work.

          Show
          salnikov Andy Salnikov added a comment - We do need better support for parsing values of the options on the command line (Nate has already started improving support for lists). One way to do it is just eval() the value (or safer ast.literal_eval() ) but that is not super-convenient in all cases as one would need to quote regular strings. I think better option would be to check field dtype and only do eval if dtype is not str . I'll try to implement that to see if that can work.
          Hide
          salnikov Andy Salnikov added a comment -

          Nate Lust, as you have already worked on that code could you also review my changes to it? I added unit test with many option types so I'm more or less confident that it does what we need now. 

          Show
          salnikov Andy Salnikov added a comment - Nate Lust , as you have already worked on that code could you also review my changes to it? I added unit test with many option types so I'm more or less confident that it does what we need now. 
          Hide
          nlust Nate Lust added a comment -

          I will look at this tonight or tomorrow morning

          Show
          nlust Nate Lust added a comment - I will look at this tonight or tomorrow morning
          Hide
          salnikov Andy Salnikov added a comment -

          Nate Lust, thanks for reviewing and closing the ticket (though as a reviewer you are only required to mark it as Reviewed ) I rebased and merged my branch, so it is indeed done now.

          Show
          salnikov Andy Salnikov added a comment - Nate Lust , thanks for reviewing and closing the ticket (though as a reviewer you are only required to mark it as Reviewed ) I rebased and merged my branch, so it is indeed done now.
          Hide
          nlust Nate Lust added a comment -

          Oh I must have hit done below reviewed, sorry about that!

          Show
          nlust Nate Lust added a comment - Oh I must have hit done below reviewed, sorry about that!

            People

            • Assignee:
              salnikov Andy Salnikov
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Nate Lust
              Watchers:
              Andy Salnikov, Jim Bosch, Nate Lust, Yusra AlSayyad
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel