Skip to content

Fix the daily pipeline failures#4084

Merged
snnn merged 10 commits intomasterfrom
snnn/d
Jun 1, 2020
Merged

Fix the daily pipeline failures#4084
snnn merged 10 commits intomasterfrom
snnn/d

Conversation

@snnn
Copy link
Contributor

@snnn snnn commented May 29, 2020

Description:

  1. Fix the nuget cpu pipeline and put code coverage pipeline back.
  2. Reduce onnx_test_runner's default logging level from WARNING to ERROR. Because there are too many log messages now.
  3. Enlarge the protobuf read buffer size for onnx_test_runner. It was missed from PR Enlarge protobuf read buffer size #4020.
    Motivation and Context
  • Why is this change required? What problem does it solve?

Currently,

  1. The nuget CPU pipeline fails with error of:
    "curl: command not found"
    while downloading azcopy which is for downloading the test data. But because I've put azcopy and all the test on the machines, so there is no need to re-download them from Azure again. To fix the problem, instead of installing curl on the machines, I deleted the data downloading step. If later on we need to add the step back, please only bring the "Download test data" step back, not the "Download azcopy". Because the azcopy command is already available at "/usr/bin/".

  2. The code coverage pipeline fails because it uses latest protobuf release and our code isn't compatible with that. I've sent the issue to Pranav. Before we get a fix, I want to migrate the pipeline to Linux and LLVM. The LLVM code coverage tool has better documentation and it is easier to configure. It can merge the coverage result from multiple runs without extra tools.

  • If it fixes an open issue, please link to the issue here.

@snnn snnn requested a review from a team as a code owner May 29, 2020 19:27
@snnn snnn changed the title Fix the pipelines Fix the daily pipeline failures May 29, 2020
@yuslepukhin
Copy link
Member

displayName: 'Clean untagged docker images'

What kind of vars this script sets and what does it download? Should we rename this script? Perhaps some comments in the header?


Refers to: tools/ci_build/github/azure-pipelines/nuget/templates/linux-set-variables-and-download.yml:3 in 79c19b8. [](commit_id = 79c19b8, deletion_comment = False)

@yuslepukhin
Copy link
Member

Would be nice to describe changes here on a high level. Fix Nuget pipeline is too general What specifically needs to be fixed.
This comments will lead to less questions being asked in the future.

logging_level = ORT_LOGGING_LEVEL_WARNING;
} else if (verbosity_option_count > 1) {
logging_level = ORT_LOGGING_LEVEL_VERBOSE;
logging_level = ORT_LOGGING_LEVEL_INFO;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about something like this for more flexibility:
logging_level = ORT_LOGGING_LEVEL_ERROR - std::min(verbosity_option_count, ORT_LOGGING_LEVEL_ERROR)
also i think verbosity_option_count can be unsigned

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. i think we can still make verbosity_option_count unsigned to ensure it won't be negative, but it currently looks like it shouldn't be

@@ -10,27 +10,30 @@

import argparse
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this is called from Linux now, maybe tools/ci_build/github/windows is not the best location. can address it in a separate PR

@snnn
Copy link
Contributor Author

snnn commented Jun 1, 2020

It's known that the nuget GPU pipeline will continue to fail. I feel sorry for that, I would like to let this change go first and put the fix in another PR if nobody disagree.

@snnn snnn merged commit 3eaec57 into master Jun 1, 2020
@snnn snnn deleted the snnn/d branch June 1, 2020 21:44
@snnn snnn mentioned this pull request Jun 2, 2020
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

Successfully merging this pull request may close these issues.

3 participants