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

Port HSC --rerun option for CmdLineTask

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: pipe_base
    • Labels:
      None
    • Story Points:
      4
    • Sprint:
      Science Pipelines DM-W16-1, Science Pipelines DM-W16-2, Science Pipelines DM-W16-3
    • Team:
      Data Release Production

      Description

      Port the HSC side's --rerun option for specifying processing inputs and outputs.

      This work should be preceded by an RFC; we've proposed implementing this option on the LSST side in the past, and it was met with some resistance as it isn't strictly necessary. We've since found it extremely convenience on the HSC side, and I think it's very much worth porting.

        Attachments

          Issue Links

            Activity

            Hide
            nlust Nate Lust added a comment -

            The LSST and HSC stacks have diverged significantly enough that replaying the commits which introduced the rerun option would be quite time consuming. I am proposing to pull over just the current code for rerun support as a new commit.

            Show
            nlust Nate Lust added a comment - The LSST and HSC stacks have diverged significantly enough that replaying the commits which introduced the rerun option would be quite time consuming. I am proposing to pull over just the current code for rerun support as a new commit.
            Hide
            nlust Nate Lust added a comment -

            Upon further review of this, I even more agree that the functionality should be ported as a new commit in its current state on HSC. Trying to replay the commit history for each commit is going to involve heavy conflict resolution, such that when you look back at the history it will tell you nothing meaningful. It will be a confusing mess of why were these things deleted when adding these other things, or "why was this taken away now, only to be introduced in subsequent patches". There will be many such things, such that in trying to preserve the editing history, we will actually be introducing a large amount of clutter and confusion

            Show
            nlust Nate Lust added a comment - Upon further review of this, I even more agree that the functionality should be ported as a new commit in its current state on HSC. Trying to replay the commit history for each commit is going to involve heavy conflict resolution, such that when you look back at the history it will tell you nothing meaningful. It will be a confusing mess of why were these things deleted when adding these other things, or "why was this taken away now, only to be introduced in subsequent patches". There will be many such things, such that in trying to preserve the editing history, we will actually be introducing a large amount of clutter and confusion
            Hide
            price Paul Price added a comment -

            I think that's fair enough.

            Show
            price Paul Price added a comment - I think that's fair enough.
            Hide
            price Paul Price added a comment -
            • Mention in the commit message that this was pulled from HSC without history.
            • A line is commented out: #namespace.output = None
            • I think _parseDirectories needs a bit more explanation, either as docstring or comments. I wonder if its goals can be achieved without writing variables in the namespace or without the need for both, e.g., rerun and rawRerun?
            • Nice addition of textwrap.dedent!
            • A pity not to include the default rerun name feature.
            • Trailing whitespace after del namespace.rawRerun.
            Show
            price Paul Price added a comment - Mention in the commit message that this was pulled from HSC without history. A line is commented out: #namespace.output = None I think _parseDirectories needs a bit more explanation, either as docstring or comments. I wonder if its goals can be achieved without writing variables in the namespace or without the need for both, e.g., rerun and rawRerun ? Nice addition of textwrap.dedent ! A pity not to include the default rerun name feature. Trailing whitespace after del namespace.rawRerun .
            Hide
            nlust Nate Lust added a comment -

            I have addressed everything except changing how variables are written to and use the namespace. I feel it is outside the scope of the ticket, and may introduce unforeseen bugs without very rigorous testing beyond the story point scale of this ticket. As per our discussion you have agreed, and this has been merged to master after a successful test of the rerun behavior on tiger, and a successful Jenkins build.

            Show
            nlust Nate Lust added a comment - I have addressed everything except changing how variables are written to and use the namespace. I feel it is outside the scope of the ticket, and may introduce unforeseen bugs without very rigorous testing beyond the story point scale of this ticket. As per our discussion you have agreed, and this has been merged to master after a successful test of the rerun behavior on tiger, and a successful Jenkins build.
            Hide
            swinbank John Swinbank added a comment - - edited

            Please make sure this is added to the W16 release notes.

            Further, the documentation here is inadequate. That's fine, given the nature of the port, but I've opened DM-4443 to ensure we come back to it later. The text provided for the release notes may, in fact, constitute adequate stack documentation – please bear that in mind while writing it!

            Show
            swinbank John Swinbank added a comment - - edited Please make sure this is added to the W16 release notes . Further, the documentation here is inadequate. That's fine, given the nature of the port, but I've opened DM-4443 to ensure we come back to it later. The text provided for the release notes may, in fact, constitute adequate stack documentation – please bear that in mind while writing it!
            Hide
            nlust Nate Lust added a comment -

            John Swinbank Can you look over the release notes for command line task and give me some feedback? I've tried to balance enough information to be appropriate for release notes, but still describe the functionality fully.

            Show
            nlust Nate Lust added a comment - John Swinbank Can you look over the release notes for command line task and give me some feedback? I've tried to balance enough information to be appropriate for release notes, but still describe the functionality fully.
            Hide
            swinbank John Swinbank added a comment -

            Thanks Nate Lust!

            Writing release notes here is actually a bit awkward. Ideally, they are a brief summary of new features, rather than user-facing documentation – where appropriate, they should contain links to where the new features are documented. However, since --rerun isn't adequately documented elsewhere (DM-4443) this is challenging.

            For now, I've slightly edited your text to make it slightly less verbose. In the longer term, as part of DM-4443, we should probably migrate these examples to the user docs and have a simple one-sentence summary & link in the release notes.

            Show
            swinbank John Swinbank added a comment - Thanks Nate Lust ! Writing release notes here is actually a bit awkward. Ideally, they are a brief summary of new features, rather than user-facing documentation – where appropriate, they should contain links to where the new features are documented. However, since --rerun isn't adequately documented elsewhere ( DM-4443 ) this is challenging. For now, I've slightly edited your text to make it slightly less verbose. In the longer term, as part of DM-4443 , we should probably migrate these examples to the user docs and have a simple one-sentence summary & link in the release notes.

              People

              Assignee:
              nlust Nate Lust
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Paul Price
              Watchers:
              Jim Bosch, John Swinbank, Nate Lust, Paul Price
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: