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

Fix timing measurement construction

    Details

    • Story Points:
      2
    • Sprint:
      Alert Production F17 - 11
    • Team:
      Alert Production

      Description

      At present, ap_verify produces few timing measurements because I was not aware of DM-11935 when I wrote the code (and the pipeline was not yet in a state where it could be tested). The code should be changed so that measurements are reliably generated, either by fixing DM-11935 or by working around it.

        Attachments

          Issue Links

            Activity

            Hide
            krzys Krzysztof Findeisen added a comment -

            Hi John Swinbank,

            Since you haven't had a chance to look at the ap_verify/ap_pipe code much, would you be willing to review this ticket? It turned out to be a combination of two bugs, but the fix for either is straightforward.

            Show
            krzys Krzysztof Findeisen added a comment - Hi John Swinbank , Since you haven't had a chance to look at the ap_verify / ap_pipe code much, would you be willing to review this ticket? It turned out to be a combination of two bugs, but the fix for either is straightforward.
            Hide
            swinbank John Swinbank added a comment -

            In my queue, but unlikely to get done this week: sorry. If you need faster turnaround, let's find another victim. If you don't get this by mid-next-week, please ping me.

            Show
            swinbank John Swinbank added a comment - In my queue, but unlikely to get done this week: sorry. If you need faster turnaround, let's find another victim. If you don't get this by mid-next-week, please ping me.
            Hide
            krzys Krzysztof Findeisen added a comment -

            I think it can wait to next week. A lot of the other ap_verify work is blocked on DM-10978 at the moment, so we won't be trying to execute test plans before then anyway.

            Show
            krzys Krzysztof Findeisen added a comment - I think it can wait to next week. A lot of the other ap_verify work is blocked on DM-10978 at the moment, so we won't be trying to execute test plans before then anyway.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Bump.

            Show
            krzys Krzysztof Findeisen added a comment - Bump.
            Hide
            krzys Krzysztof Findeisen added a comment -

            John Swinbank, can I please get a review (or a recommendation for an alternate reviewer)? This ticket is blocking Chris Morrison's work on DM-11155.

            Show
            krzys Krzysztof Findeisen added a comment - John Swinbank , can I please get a review (or a recommendation for an alternate reviewer)? This ticket is blocking Chris Morrison 's work on DM-11155 .
            Hide
            swinbank John Swinbank added a comment -

            Sorry for being slow to look at this.

            I think the code as written is fine, certainly as it applies to ap_verify. I'm worried that the (pre-existing) code in ap_pipe is intrinsically hard to understand: I think that what you've done is reasonable, but I'm not confident I've fully understood the intent of the codebase.

            There is one trivial change (removing the doReturnResults argument) which I think you should make before merging. The rest of it is ok to go as is (but if you can explain to me why the code uses parseAndRun rather than just run, I'd be interested).

            Show
            swinbank John Swinbank added a comment - Sorry for being slow to look at this. I think the code as written is fine, certainly as it applies to ap_verify. I'm worried that the (pre-existing) code in ap_pipe is intrinsically hard to understand: I think that what you've done is reasonable, but I'm not confident I've fully understood the intent of the codebase. There is one trivial change (removing the doReturnResults argument) which I think you should make before merging. The rest of it is ok to go as is (but if you can explain to me why the code uses parseAndRun rather than just run , I'd be interested).
            Hide
            krzys Krzysztof Findeisen added a comment -

            Chris Morrison, you should be able to rebase the changes to the timing measurement code now.

            Show
            krzys Krzysztof Findeisen added a comment - Chris Morrison , you should be able to rebase the changes to the timing measurement code now.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel