Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding extra commands element into HelixWorkItem #11157

Closed
cincuranet opened this issue Oct 6, 2022 · 6 comments
Closed

Adding extra commands element into HelixWorkItem #11157

cincuranet opened this issue Oct 6, 2022 · 6 comments

Comments

@cincuranet
Copy link
Contributor

As part of dotnet/performance#2416 I need to extend HelixWorkItem with extra commands element. These commands will be run locally as the payload (CorrelationPayloadDirectory) is being prepared. Obviously PreCommands is no good, that's executed as part of the executed test/benchmark scenario. I have a raw PoW prepared, but before I create PR I'd like to discuss some details to properly fit into existing code.

The new HelixWorkItem would looks like this (I'm using JiriCommand for now, before we finish good naming):

<HelixWorkItem Include="Hello">
  <Command>aaa</Command>
  <JiriCommand>foo</JiriCommand>
</HelixWorkItem>
<HelixWorkItem Include="Hello2">
  <Command>bbb</Command>
  <JiriCommand>bar</JiriCommand>
</HelixWorkItem>

Now couple of questions:

  1. What would be good name for JiriCommand? PrepareCommand seems to a good one to me.
  2. Should it be plural like i.e. PreCommands and hence support splitting? Or single command like i.e. Command? Single command/singular seems enough to me.
  3. Is it worth adding support also for HelixJiriCommand[s] (like HelixPreCommands)? If so, very likely to keep it consistent the answer to question 2 is "plural".
  4. To execute the commands, is Microsoft.DotNet.Helix.Sdk.targets a good place to add target? I think i.e. Microsoft.DotNet.Helix.Sdk.MonoQueue.targets is too "late" in the processing (although it would work as well).
  5. Is it worth extending also eng\common\build.ps1 and Build.proj with a switch to execute above mentioned target? Or leave it for manual explicit execution only.

@sblom @DrewScoggins

@premun
Copy link
Member

premun commented Oct 7, 2022

These commands will be run locally as the payload (CorrelationPayloadDirectory) is being prepared

Can you please elaborate on this? Can you maybe describe an actual scenario that you need this for?
I don't understand what it is you cannot achieve via already existing things.

@cincuranet
Copy link
Contributor Author

cincuranet commented Oct 7, 2022

Sure. Here's an example. For scenarios we always build and publish the app before running the scenario test. Some scenarios have same binary, only different things are measured, thus doing always build and publish makes it slower (and also wastes resources). The goal is to improve the process by doing build and publish in advance and including it into the payload. Eventually we'd like to use the same approach for places where doing the build as part of work item is challenging because multiple things need to properly align - like for example MAUI.

Does that make sense?

It's not a deal breaker if it's not a good fit here. I can implement it only in proj files in dotnet/performance, but makes more sense to me to have it upstream.

@premun
Copy link
Member

premun commented Oct 7, 2022

I am still not sure I follow what do you need to happen when that you cannot already do with the current setup.

It seems like you want some things to happen before you define the <HelixCorrelationPayload> item(s)?

@cincuranet
Copy link
Contributor Author

cincuranet commented Oct 7, 2022

It seems like you want some things to happen before you define the item(s)?

Correct.

I am still not sure I follow what do you need to happen when that you cannot already do with the current setup.

We want to keep everything in one place, in HelixWorkItem. Instead of having the "prepare" part in some other place.

@premun
Copy link
Member

premun commented Oct 7, 2022

But why not handle that by MSBuild targets that would depend on each other? It can still very much happen in that one proj file.

In my opinion, you have to give Helix SDK some folder that has to be in some shape before it sends it to Helix. I don't understand why you would prepare just half of the things first, then invoke Helix SDK and then want to finish the preparation later inside Helix SDK? Why not prepare it before and be done?

Seems like you still have a "prepare" part somewhere so why not make it complete?

@cincuranet
Copy link
Contributor Author

Solved offline. Not good fit for here.

@cincuranet cincuranet closed this as not planned Won't fix, can't repro, duplicate, stale Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants