-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Tox + code coverage #150
Merged
Merged
Tox + code coverage #150
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
f69a2ca
Merge pull request #2 from microsoft/master
algattik 7b370c1
Merge remote-tracking branch 'upstream/master'
algattik 8556d62
.
algattik 979d846
.
algattik 08e76e3
Update requirements.txt
algattik f25590a
Update azdo-ci-build-train.yml
algattik 9e5585f
Squashed commit of the following:
algattik 3ab3230
Merge branch 'master' of https://github.com/microsoft/MLOpsPython
algattik d29a008
Merge branch 'master' into algattik/code-coverage
algattik 2b7707f
Update Dockerfile
algattik 26e1a01
Update tox.ini
algattik ef14bc8
Update development_setup.md
algattik b4268af
Reverted Dockerfile
algattik 6596317
Merge branch 'master' into algattik/code-coverage
eedorenko d7308fc
Merge remote-tracking branch 'upstream/master' into algattik/code-cov…
algattik dc00144
Do not use tox virtualenv
algattik File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,26 +1,20 @@ | ||
# this pipeline should be ignored for now | ||
parameters: | ||
pipelineType: 'training' | ||
|
||
steps: | ||
- script: | | ||
flake8 --output-file=$(Build.BinariesDirectory)/lint-testresults.xml --format junit-xml | ||
workingDirectory: '$(Build.SourcesDirectory)' | ||
displayName: 'Run code quality tests' | ||
enabled: 'true' | ||
|
||
- script: | | ||
pytest --junitxml=$(Build.BinariesDirectory)/unit-testresults.xml $(Build.SourcesDirectory)/tests/unit | ||
displayName: 'Run unit tests' | ||
enabled: 'true' | ||
env: | ||
SP_APP_SECRET: '$(SP_APP_SECRET)' | ||
tox | ||
displayName: 'Linting & unit tests' | ||
|
||
- task: PublishTestResults@2 | ||
condition: succeededOrFailed() | ||
inputs: | ||
testResultsFiles: '$(Build.BinariesDirectory)/*-testresults.xml' | ||
testResultsFiles: '*-testresults.xml' | ||
testRunTitle: 'Linting & Unit tests' | ||
failTaskOnFailedTests: true | ||
displayName: 'Publish linting and unit test results' | ||
enabled: 'true' | ||
displayName: 'Publish test results' | ||
|
||
- task: PublishCodeCoverageResults@1 | ||
displayName: 'Publish coverage report' | ||
condition: succeededOrFailed() | ||
inputs: | ||
codeCoverageTool: Cobertura | ||
summaryFileLocation: 'coverage.xml' | ||
failIfCoverageEmpty: true |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
## Development environment setup | ||
|
||
### Setup | ||
|
||
Please be aware that the local environment also needs access to the Azure subscription so you have to have Contributor access on the Azure ML Workspace. | ||
|
||
In order to configure the project locally, create a copy of `.env.example` in the root directory and name it `.env`. Fill out all missing values and adjust the existing ones to suit your requirements. | ||
|
||
### Installation | ||
|
||
[Install the Azure CLI](https://docs.microsoft.com/en-us/cli/azure/install-azure-cli). The Azure CLI will be used to log you in interactively. | ||
|
||
Create a virtual environment using [venv](https://docs.python.org/3/library/venv.html), [conda](https://docs.conda.io/projects/conda/en/latest/user-guide/tasks/manage-environments.html) or [pyenv-virtualenv](https://github.com/pyenv/pyenv-virtualenv). | ||
|
||
Here is an example for setting up and activating a `venv` environment with Python 3: | ||
|
||
``` | ||
python3 -mvenv .venv | ||
source .venv/bin/activate | ||
``` | ||
|
||
Install the required Python modules in your virtual environment. | ||
|
||
``` | ||
pip install -r environment_setup/requirements.txt | ||
``` | ||
|
||
### Running local code | ||
|
||
To run your local ML pipeline code on Azure ML, run a command such as the following (in bash, all on one line): | ||
|
||
``` | ||
export BUILD_BUILDID=$(uuidgen); python ml_service/pipelines/build_train_pipeline.py && python ml_service/pipelines/run_train_pipeline.py | ||
``` | ||
|
||
BUILD_BUILDID is a variable used to uniquely identify the ML pipeline between the | ||
`build_train_pipeline.py` and `run_train_pipeline.py` scripts. In Azure DevOps it is | ||
set to the current build number. In a local environment, we can use a command such as | ||
`uuidgen` so set a different random identifier on each run, ensuring there are | ||
no collisions. | ||
|
||
### Local testing | ||
|
||
Before committing, run `tox` to execute linter and unit test checks. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,11 +4,20 @@ LABEL org.label-schema.vendor = "Microsoft" \ | |
org.label-schema.url = "https://hub.docker.com/r/microsoft/mlopspython" \ | ||
org.label-schema.vcs-url = "https://github.com/microsoft/MLOpsPython" | ||
|
||
|
||
# Utilities for package installation | ||
RUN apt-get update && apt-get install -y \ | ||
curl \ | ||
apt-transport-https \ | ||
gcc | ||
|
||
COPY environment_setup/requirements.txt /setup/ | ||
|
||
RUN apt-get update && apt-get install gcc -y && pip install --upgrade -r /setup/requirements.txt && \ | ||
conda install -c r r-essentials | ||
# Install dotnet runtime used to generate code coverage report | ||
RUN curl -O https://packages.microsoft.com/config/ubuntu/19.04/packages-microsoft-prod.deb \ | ||
&& dpkg -i packages-microsoft-prod.deb \ | ||
&& apt-get update && apt-get install -y dotnet-runtime-3.1 | ||
|
||
# Packages for running R | ||
RUN conda install -c r r-essentials | ||
|
||
CMD ["python"] | ||
# Install Python modules | ||
COPY environment_setup/requirements.txt /setup/ | ||
RUN pip install --upgrade -r /setup/requirements.txt | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider reducing the number of image layers by combining RUN commands. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated to reduce RUN steps |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,12 @@ | ||
pytest>=5.3 | ||
pytest-cov>=2.8.1 | ||
requests>=2.22 | ||
numpy>=1.17 | ||
pandas>=0.25 | ||
scikit-learn>=0.21.3 | ||
azureml-sdk>=1.0 | ||
python-dotenv>=0.10.3 | ||
flake8>=3.7 | ||
flake8_formatter_junit_xml | ||
flake8_formatter_junit_xml>=0.0.6 | ||
tox>=3.14.3 | ||
azure-cli==2.0.76 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
[tox] | ||
minversion = 3.14.3 | ||
skipsdist = True | ||
|
||
[flake8] | ||
# ignore obsolete warning | ||
ignore = W503 | ||
exclude = .git,__pycache__,.venv,.tox,**/site-packages/**/*.py,**/lib/**.py,**/bin/**.py | ||
|
||
[pytest] | ||
junit_family = legacy | ||
|
||
[testenv] | ||
whitelist_externals = | ||
flake8 | ||
pytest | ||
passenv = HOME | ||
commands = | ||
flake8 --output-file=lint-testresults.xml --format junit-xml | ||
pytest tests/unit --cov=diabetes_regression --cov-report=xml --junitxml=unit-testresults.xml |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need
dotnet-runtime-3.1
? I assume code coverage will still show up in Azure DevOps, but we'll just get the warning of no auto-generated html reports, no?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without dotnet runtime we only get the report as an artifact.
With dotnet runtime we get an an embedded HTML report
this can be achieved by:
Given the impact of 1 or 2 in terms of build time and the limited benefit of the embedded report, I will update the PR to go for neither option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another idea would be to add 2. commented out in the pipeline if people want to enable the code coverage html report
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Note that we will see this warning:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. There is no update to the Dockerfile anymore, but the mlopspython image must still be rebuilt because of updates to requirements.txt.
I tested installing dotnet core but coverage report generation then fails with a missing libicu library, and sudo is not available to install it. I suggest investigating this at a later point.