Use Azure ML Environments and add code upload option

[tomasvanpottelbergh]

First attempt at PR to resolve #14. Tests to be updated after first feedback.

[marrrcin]

So it basically uploads all of the files in your current directory and executes the code in the provided environment, right? If it works, then it's fine to me as long as we preserve the standard flow with the docker.

As for environment creation verification - maybe we can check whether the checksum of local Dockerfile matches the checksum of the environment's Dockerfile (if it exists), if not, we can trigger the build via

from azureml.core import Environment
custom_env = Environment.from_dockerfile(env_name, "./Dockerfile")
# check if environment exists + compare dockerfiles, if not run below:
custom_env = custom_env.register(ws)
build_result = custom_env.build(ws)
build_result.wait_for_completion()

AML SDK API.

@em-pe what are your thoughts on this?

[tomasvanpottelbergh]

So it basically uploads all of the files in your current directory and executes the code in the provided environment, right? If it works, then it's fine to me as long as we preserve the standard flow with the docker.

Indeed. It does preserve the current Docker workflow, although the default would be to have code upload. I think this makes sense as this is how Azure ML Jobs are used in the documentation and examples.

As for environment creation verification - maybe we can check whether the checksum of local Dockerfile matches the checksum of the environment's Dockerfile (if it exists), if not, we can trigger the build via AML SDK API.

That would indeed work if the Environment is created just using a Dockerfile. However, Azure ML Environments can be created in several ways (curated, Docker images in registry, Dockerfile, Docker build context, conda YAML file, and possibly others). Supporting a diff checking for all these methods would be hard to impossible. Just as an example: for the Docker build context you would need to check whether any of the files used by the Dockerfile changed since the last time you created the Environment.

[marrrcin]

OK, we're fine with the code-upload workflow, but let's keep it as an alternative, leaving the docker workflow as default (which would be coherent across all of our plugins).
Please proceed with this PR, we will then test it and merge it. It seems like some of the unit tests are failing in the CI:

  ==================================== ERRORS ====================================
  ______________________ ERROR collecting tests/test_cli.py ______________________
  ImportError while importing test module '/home/runner/work/kedro-azureml/kedro-azureml/tests/test_cli.py'.
  Hint: make sure your test modules/packages have valid Python names.
  Traceback:
  /opt/hostedtoolcache/Python/3.10.6/x64/lib/python3.10/importlib/__init__.py:126: in import_module
      return _bootstrap._gcd_import(name[level:], package, level)
  tests/test_cli.py:14: in <module>
      from kedro_azureml.constants import FILL_IN_DOCKER_IMAGE
  E   ImportError: cannot import name 'FILL_IN_DOCKER_IMAGE' from 'kedro_azureml.constants' (/home/runner/work/kedro-azureml/kedro-azureml/kedro_azureml/constants.py)
  
  ---------- coverage: platform linux, python 3.10.6-final-0 -----------
  Name                             Stmts   Miss Branch BrPart  Cover   Missing
  ----------------------------------------------------------------------------
  kedro_azureml/__init__.py            3      0      0      0   100%
  kedro_azureml/cli.py                85     31     18      2    54%   29, 35->exit, 46-47, 72-88, 100->exit, 143-177, 214-217, 250-259
  kedro_azureml/cli_functions.py      21     12      6      0    33%   13-32, 36-42
  kedro_azureml/client.py             43     27     10      0    34%   19-39, 44-45, 53-80
  kedro_azureml/config.py             23      0      8      0   100%
  kedro_azureml/constants.py           3      0      0      0   100%
  kedro_azureml/datasets.py           32     15     10      0    45%   22-27, 31, 35, 41-45, 48-52, 55
  kedro_azureml/generator.py          73     50     20      0    27%   29-34, 37-66, 69-72, 75, 78, 87, 113-125, 136-155, 158-166
  kedro_azureml/runner.py             22      9      4      0    58%   16-18, 29-34, 39
  kedro_azureml/utils.py              27     10      4      0    68%   21-25, 30, 33-37, 40
  ----------------------------------------------------------------------------
  TOTAL                              332    154     80      2    49%
  Coverage XML written to file coverage.xml
  
  =========================== short test summary info ============================
  ERROR tests/test_cli.py
  !!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
  =============================== 1 error in 2.43s ===============================
  Error: ERROR: InvocationError for command /opt/hostedtoolcache/Python/3.10.6/x64/bin/poetry run pytest --cov kedro_azureml --cov-report xml --cov-report term-missing --ignore=venv --verbose -W ignore::marshmallow.warnings.RemovedInMarshmallow4Warning (exited with code 2)

[marrrcin]

@tomasvanpottelbergh any updates? :)

[tomasvanpottelbergh]

Sorry, it's been pretty busy. I hope to work on this on Friday.

[marrrcin]

Pre-commits are not passing:

isort....................................................................Passed
black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted kedro_azureml/cli_functions.py

All done! ✨ 🍰 ✨
1 file reformatted, 24 files left unchanged.

Flake8...................................................................Passed
Error: Process completed with exit code 1.

Please make sure that you're running pre-commit as well as unit tests locally, before pushing the code.

[tomasvanpottelbergh]

Sorry for the long wait... The unit tests now pass locally. Would be great if you could test and review!

[tomasvanpottelbergh]

@marrrcin could it be that there's something wrong with the SonarCloud config?

[marrrcin]

Thanks for your contribution @tomasvanpottelbergh, we will test it this week!

SonarCloud will not pass on PRs due to missing key in the configuration.

[marrrcin]

@tomasvanpottelbergh FYI: I had to restore some of the functionality, because your PR broke the backward compatibility.
Also, I've went through the quickstart with code upload you introduced and there were a few uncovered issues, especially w.r.t environments and the file exclusions. I've added some functionality and meaningful log messages to guide the users better through this process, because without them, the quickstart with Spaceflights start was impossible to run.

[tomasvanpottelbergh]

Thanks for accepting the PR and cleaning it up! It was indeed not intended to be backwards compatible, since I thought breaking the API was preferable over complicating the code at this point, but that's of course up to you to decide.

I noticed you mention that code upload is the default in the updated quickstart, but I actually reverted this as requested in 65e7841. So you would need to set code_directory: "." in the config template if you want it to be the default.

[marrrcin]

@tomasvanpottelbergh I have a weird issue with the az ml environment create, I pass --build-context as well as --dockerfile-path but in the Azure ML Studio, the Environment Builds fails almost immediately after registering with absolutely no build logs. Have you observed similar behaviour?

[tomasvanpottelbergh]

We are having a similar issue when running this command from an Azure ML Compute Instance. Is that what you're doing? Locally it worked for me, but we should report this bug to the Azure SDK repo.

[marrrcin]

I've created Azure/azure-cli-extensions#5560 for that.