Skip to content

Comments

enable build step timeout also some code cleanup#5122

Merged
frank-dong-ms-zz merged 4 commits intodotnet:masterfrom
frank-dong-ms-zz:small-fix
May 14, 2020
Merged

enable build step timeout also some code cleanup#5122
frank-dong-ms-zz merged 4 commits intodotnet:masterfrom
frank-dong-ms-zz:small-fix

Conversation

@frank-dong-ms-zz
Copy link
Contributor

@frank-dong-ms-zz frank-dong-ms-zz commented May 13, 2020

  1. set the build steps to timeout at 20 minutes
  2. remove MatrixFactorizationFact
  3. remove RetryFact and related class

@frank-dong-ms-zz frank-dong-ms-zz requested a review from a team as a code owner May 13, 2020 22:26
@frank-dong-ms-zz frank-dong-ms-zz requested a review from harishsk May 13, 2020 22:26
@frank-dong-ms-zz frank-dong-ms-zz requested a review from mstfbl May 13, 2020 22:28
@harishsk
Copy link
Contributor

harishsk commented May 13, 2020

Are you sure you don't need the retry-tests ability?
I am thinking it may be good to have it in the code base in case some other similar bug crops up and we need a retry ability to either debug or temporarily work around. #Resolved

@frank-dong-ms-zz
Copy link
Contributor Author

We have IterationData, SkipInCI and RunSpecificTests that currently good enough for investigate random test failures.
During our investigation Retry Fact is not providing much value, this Fact can only be good for temporary workaround for issue like race condition.
Given the current shape of CI I think it is time to remove the Retry Fact.


In reply to: 628280004 [](ancestors = 628280004)

@harishsk
Copy link
Contributor

Sounds good.


In reply to: 628283384 [](ancestors = 628283384,628280004)

@codecov
Copy link

codecov bot commented May 14, 2020

Codecov Report

Merging #5122 into master will increase coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5122      +/-   ##
==========================================
+ Coverage   75.55%   75.60%   +0.04%     
==========================================
  Files         995      990       -5     
  Lines      179318   179202     -116     
  Branches    19298    19287      -11     
==========================================
- Hits       135485   135479       -6     
+ Misses      38567    38461     -106     
+ Partials     5266     5262       -4     
Flag Coverage Δ
#Debug 75.60% <ø> (+0.04%) ⬆️
#production 71.50% <ø> (ø)
#test 88.86% <ø> (+0.22%) ⬆️
Impacted Files Coverage Δ
test/Microsoft.ML.Functional.Tests/Evaluation.cs 100.00% <ø> (ø)
...ests/TrainerEstimators/MatrixFactorizationTests.cs 97.59% <ø> (+0.51%) ⬆️
...rosoft.ML.AutoML/ColumnInference/TextFileSample.cs 59.60% <0.00%> (-5.97%) ⬇️
...icrosoft.ML.AutoML/Experiment/SuggestedPipeline.cs 88.65% <0.00%> (-4.13%) ⬇️
...L.AutoML/TrainerExtensions/TrainerExtensionUtil.cs 85.15% <0.00%> (-1.75%) ⬇️
src/Microsoft.ML.AutoML/Sweepers/Parameters.cs 85.16% <0.00%> (+0.84%) ⬆️
....ML.AutoML/PipelineSuggesters/PipelineSuggester.cs 86.55% <0.00%> (+5.88%) ⬆️
...c/Microsoft.ML.FastTree/Utils/ThreadTaskManager.cs 100.00% <0.00%> (+20.51%) ⬆️

@codecov
Copy link

codecov bot commented May 14, 2020

Codecov Report

Merging #5122 into master will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5122      +/-   ##
==========================================
+ Coverage   75.55%   75.60%   +0.05%     
==========================================
  Files         995      990       -5     
  Lines      179318   179204     -114     
  Branches    19298    19287      -11     
==========================================
+ Hits       135485   135494       +9     
+ Misses      38567    38450     -117     
+ Partials     5266     5260       -6     
Flag Coverage Δ
#Debug 75.60% <ø> (+0.05%) ⬆️
#production 71.51% <ø> (+0.01%) ⬆️
#test 88.85% <ø> (+0.22%) ⬆️
Impacted Files Coverage Δ
test/Microsoft.ML.Functional.Tests/Evaluation.cs 100.00% <ø> (ø)
...ests/TrainerEstimators/MatrixFactorizationTests.cs 97.07% <ø> (ø)
...icrosoft.ML.AutoML/Experiment/SuggestedPipeline.cs 88.65% <0.00%> (-4.13%) ⬇️
src/Microsoft.ML.AutoML/Sweepers/Parameters.cs 85.16% <0.00%> (+0.84%) ⬆️
src/Microsoft.ML.Sweeper/AsyncSweeper.cs 73.97% <0.00%> (+1.36%) ⬆️
....ML.AutoML/PipelineSuggesters/PipelineSuggester.cs 86.55% <0.00%> (+5.88%) ⬆️
...c/Microsoft.ML.FastTree/Utils/ThreadTaskManager.cs 100.00% <0.00%> (+20.51%) ⬆️

@frank-dong-ms-zz frank-dong-ms-zz merged commit a2406f6 into dotnet:master May 14, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants