Skip to content
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

[RPD-250] Move _show_terraform_outputs() into provision #188

Merged

Conversation

swells2020
Copy link
Contributor

@swells2020 swells2020 commented Aug 8, 2023

This PR moves the _show_terraform_outputs function out of the azure_runner class and into the matcha_ml.core.core.py module. The _show_terraform_outputs function is a generic utility function for the core provision function and, as such, should be contained in the same module. The return value of the azure_runner's provision method has been updated to pass a MatchaStateService instance, instantiated with the terraform outputs of the provisioningback to the coreprovision` function.

A test has been added for the _show_terraform_outputs function.

Checklist

Please ensure you have done the following:

  • I have read the CONTRIBUTING guide.
  • I have updated the documentation if required.
  • I have added tests which cover my changes.

Type of change

Tick all those that apply:

  • Bug Fix (non-breaking change, fixing an issue)
  • New feature (non-breaking change to add functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Other (add details above)

@swells2020 swells2020 changed the title [WIP] updates core and azure_runner [RPD-250] Move _show_terraform_outputs() into provision Aug 9, 2023
@swells2020 swells2020 self-assigned this Aug 9, 2023
Copy link
Member

@Christopher-Norman Christopher-Norman left a comment

Choose a reason for hiding this comment

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

Added comments

tests/test_cli/test_provision.py Outdated Show resolved Hide resolved
src/matcha_ml/runners/azure_runner.py Show resolved Hide resolved
Copy link
Member

@Christopher-Norman Christopher-Norman left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@KirsoppJ KirsoppJ left a comment

Choose a reason for hiding this comment

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

LGTM

@swells2020 swells2020 merged commit ff520a0 into fixes/RPD-278 Aug 10, 2023
2 checks passed
@JonoCX JonoCX mentioned this pull request Aug 14, 2023
7 tasks
JonoCX added a commit that referenced this pull request Aug 15, 2023
* [RPD-287] ZenML version inference for zenserver (#180)

* various changes

* change variable names to make things more clear

* shubham's comment

* test for latest

* update existing tests

* update existing tests

* shubham latest comment

* add gitflow notation branches to ci (#183)

* remove old, commented out code

* [RPD-260] Add an object to handle the `matcha.config.json` file. (#184)

* adds logic and tests for matcha config module

* updates docstrings

* adds tests and implements config object throughout matcha

* updated for pr comments

* updates docstring

* fixes ci

* updates for comments

* [RPD-249] Refactor `analytics_service` to simplify tracking decorator (#181)

* refactor analytics service

* adds tests and updates for comments

* updates tests

* updates for comments

* extracts _get_state_uuid logic into private function

* removes old _get_state_uuid logic

* updates _execute_analytics_event_function

* extracts _time_event logic

* adds _post_event logic

* adds tests

* adds dataclass

* updates tests

* [RPD-236] Improved approach to check ignored folder when uploading and downloading (#186)

* [RPD-250] Move `_show_terraform_outputs()` into `provision` (#188)

* updates core and azure_runner

* adds test

* updated for comments

* updates test fixtures

* [RPD-259] Refactor `build_state_from_terraform_output` within `matcha_state.py` to use objects defined within matcha_state.py (#190)

* updates build_state_from_terraform_outputs

* updates for comments

* fixes tests

* removes print statement

* updates get_component

* updates get_component

* updates for comments

* [RPD-290] Update MatchaConfig object to not throw an error when a matching property/component is not found (#192)

---------

Co-authored-by: KirsoppJ <40233184+KirsoppJ@users.noreply.github.com>
Co-authored-by: Callum Wells <68609181+swells2020@users.noreply.github.com>
@Christopher-Norman Christopher-Norman deleted the RPD-250-move-_show_terraform_output-into-provision branch August 23, 2023 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants