search

dotnet / machinelearning

ML.NET is an open source and cross-platform machine learning framework for .NET.
https://dot.net/ml
MIT License
9.02k stars 1.88k forks source link

Don't include the SDK in our helix payload #6918

Closed ericstj closed 9 months ago

ericstj commented 9 months ago

I noticed that the tests included the latest SDK - including the host - in our helix payloads.

This is a large amount of unnecessary downloads and it also makes it so we use the latest host on the older frameworks which can fail when the latest host drops support for distros.

Since our tests shouldn't need the full CLI, remove this from our helix payloads.

We'll instead get just the runtime we need through AdditionalDotNetPackage

ericstj commented 9 months ago

I noticed that we might not get dotnet added to the path: https://github.com/dotnet/arcade/blob/c0c425d9b85b125bbaf59581639355f1d2b99149/src/Microsoft.DotNet.Helix/Sdk/tools/dotnet-cli/DotNetCli.targets#L15

So we might need to tweak our command line for executing the tests: https://github.com/dotnet/machinelearning/blob/5483ba93c591367e9465884ca23feab79f4bf1f0/eng/helix.proj#L151

ericstj commented 9 months ago

Yep, right on time as I discovered this the tests hit it. I do see this is having the desired effect (runtime added, but no SDK): https://helixde107v0xdeko0k025g8.blob.core.windows.net/helix-job-f33c5c7d-ade1-462f-bbf7-9c7eceea97510e66aa157474e3aad/job-list-92e85b36-8b0e-416c-b245-155dc7a13ad0.json?helixlogtype=result

Now to get dotnet back on the path.

codecov[bot] commented 9 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (efab011) 68.79% compared to head (51a7461) 68.79%. Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #6918 +/- ## ======================================= Coverage 68.79% 68.79% ======================================= Files 1249 1249 Lines 249431 249431 Branches 25510 25510 ======================================= + Hits 171604 171606 +2 Misses 71214 71214 + Partials 6613 6611 -2 ``` | [Flag](https://app.codecov.io/gh/dotnet/machinelearning/pull/6918/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | Coverage Δ | | |---|---|---| | [Debug](https://app.codecov.io/gh/dotnet/machinelearning/pull/6918/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `68.79% <ø> (+<0.01%)` | :arrow_up: | | [production](https://app.codecov.io/gh/dotnet/machinelearning/pull/6918/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `63.25% <ø> (+<0.01%)` | :arrow_up: | | [test](https://app.codecov.io/gh/dotnet/machinelearning/pull/6918/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet) | `88.49% <ø> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet#carryforward-flags-in-the-pull-request-comment) to find out more. [see 2 files with indirect coverage changes](https://app.codecov.io/gh/dotnet/machinelearning/pull/6918/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dotnet)