Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Labels:None
-
Story Points:8
-
Epic Link:
-
Sprint:AP S22-6 (May)
-
Team:Alert Production
-
Urgent?:No
Description
Currently, ApPipe.yaml is considered the "base" AP pipeline; it is included by ApVerify.yaml, which is in turn included by ApVerifyWithFakes.yaml. However, this design causes problems for ApVerifyWithFakes, which has a significantly different structure (in particular, a distinction between steps done with and without fakes) and its own rules for which metrics instrument which tasks.
The problem can be mitigated by splitting ApPipe.yaml into two pipelines: one which is a dependency of ApVerify.yaml, and a new ApPipeWithFakes.yaml that is a dependency of ApVerifyWithFakes.yaml. ApPipe.yaml and ApPipeWithFakes.yaml will not depend on each other, allowing each to have the labels, contracts, and other elements that are appropriate for that pipeline.
Configurations that should be shared between the two pipelines should be factored out into their own configs, consistent with RFC-775 guidelines.
Attachments
Issue Links
Activity
Krzysztof Findeisen
created issue -
Krzysztof Findeisen
made changes -
| Field | Original Value | New Value |
|---|---|---|
| Description |
Currently, {{ApPipe.yaml}} is considered the "base" AP pipeline; it is included by {{ApVerify.yaml}}, which is in turn included by {{ApVerifyWithFakes.yaml}}. However, this design causes problems for {{ApVerifyWithFakes}}, which has a significantly different structure (in particular, a distinction between steps done with and without fakes) and its own rules for which metrics instrument which tasks.
The problem can be mitigated by splitting {{ApPipe.yaml}} into two pipelines: one which is a dependency of {{ApVerify.yaml}}, and a new {{ApPipeWithFakes.yaml}} that is a dependency of {{ApVerifyWithFakes.yaml}}. {{ApPipe.yaml}} and {{ApPipeWithFakes.yaml}} will not depend on each other, allowing each to have the labels, contracts, and other elements that are appropriate for that pipeline. |
Currently, {{ApPipe.yaml}} is considered the "base" AP pipeline; it is included by {{ApVerify.yaml}}, which is in turn included by {{ApVerifyWithFakes.yaml}}. However, this design causes problems for {{ApVerifyWithFakes}}, which has a significantly different structure (in particular, a distinction between steps done with and without fakes) and its own rules for which metrics instrument which tasks.
The problem can be mitigated by splitting {{ApPipe.yaml}} into two pipelines: one which is a dependency of {{ApVerify.yaml}}, and a new {{ApPipeWithFakes.yaml}} that is a dependency of {{ApVerifyWithFakes.yaml}}. {{ApPipe.yaml}} and {{ApPipeWithFakes.yaml}} will not depend on each other, allowing each to have the labels, contracts, and other elements that are appropriate for that pipeline. Configurations that should be shared between the two pipelines should be factored out into their own configs, consistent with RFC-775 guidelines. |
Krzysztof Findeisen
made changes -
| Rank | Ranked higher |
See also DM-32988, which formalizes the question of multi-tract processing and will do some pipeline cleanup of its own.
Krzysztof Findeisen
added a comment - - edited See also DM-32988 , which formalizes the question of multi-tract processing and will do some pipeline cleanup of its own. Krzysztof Findeisen
made changes -
Krzysztof Findeisen
made changes -
| Rank | Ranked higher |
| Sprint | AP S22-4 (March) [ 1148 ] |
| Rank | Ranked higher |
| Sprint | AP S22-4 (March) [ 1148 ] | AP S22-5 (April) [ 1156 ] |
| Rank | Ranked lower |
Krzysztof Findeisen
made changes -
| Rank | Ranked higher |
Krzysztof Findeisen
made changes -
| Sprint | AP S22-5 (April) [ 1156 ] | AP S22-6 (May) [ 1161 ] |
Krzysztof Findeisen
made changes -
| Rank | Ranked higher |
Krzysztof Findeisen
made changes -
| Sprint | AP S22-6 (May) [ 1161 ] | AP S22-5 (April) [ 1156 ] |
Krzysztof Findeisen
made changes -
| Rank | Ranked lower |
Krzysztof Findeisen
made changes -
| Status | To Do [ 10001 ] | In Progress [ 3 ] |
| Story Points | 4 | 8 |
| Sprint | AP S22-5 (April) [ 1156 ] | AP S22-6 (May) [ 1161 ] |
| Rank | Ranked higher |
Ian Sullivan asked that this issue be merged at least one day after DM-34254, as both have changes that could affect completeness metrics.
Krzysztof Findeisen
added a comment - Ian Sullivan asked that this issue be merged at least one day after DM-34254 , as both have changes that could affect completeness metrics. Krzysztof Findeisen
made changes -
Thanks for agreeing to look at this, Meredith Rawls! The biggest changes ended up being in ap_pipe; I recommend looking at those and ap_verify by individual commits.
Krzysztof Findeisen
added a comment - Thanks for agreeing to look at this, Meredith Rawls ! The biggest changes ended up being in ap_pipe ; I recommend looking at those and ap_verify by individual commits. Krzysztof Findeisen
made changes -
| Reviewers | Meredith Rawls [ mrawls ] | |
| Status | In Progress [ 3 ] | In Review [ 10004 ] |
Thank you for fixing this up to be much more self-consistent, and to implement a less onerous DM-30210 workaround. Most of the GitHub comments are minor, and provided you can please convince me the camera-specific ApPipeWithFakes pipelines are importing all the right things from all the right places, I am very happy to have you merge this.
| Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
Krzysztof Findeisen
made changes -
| Resolution | Done [ 10000 ] | |
| Status | Reviewed [ 10101 ] | Done [ 10002 ] |
Krzysztof Findeisen
added a comment - Ok, everything's merged. This should unblock DM-34710 and DM-34623 . Krzysztof Findeisen
made changes -
DM-32245added a ApPipeMultiTractFakes.yaml; use this as the basis instead of ApVerifyWithFakes.yaml (there's still some work to do on this ticket, as the current file depends on ApPipe, and ApPipe is not yet multi-tract).