-
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
Tox + code coverage #150
Changes from 11 commits
f69a2ca
7b370c1
8556d62
979d846
08e76e3
f25590a
9e5585f
3ab3230
d29a008
2b7707f
26e1a01
ef14bc8
b4268af
6596317
d7308fc
dc00144
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
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/build-image/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. |
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 | ||
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. Do we really need 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. 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 commentThe 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 commentThe 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 commentThe 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 commentThe 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. |
||
|
||
# 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 |
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 |
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 |
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.
requirements.txt is in environment_setup folder
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.
It should be "pip install -r environment_setup/requirements.txt"
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, apologies