[RPD-284] Update AzureRunner to select the Terraform files corresponding to the stack name in the config file #187

Christopher-Norman · 2023-08-08T09:34:42Z

This PR:

Renames the resources Terraforms file to default.
Adds functionality within core.py provision function to collect the stack name and use that as a pointer to the specified set of Terraform files.
If this value does not exist it will create it and use the default stack.

This PR also fixes a bug where an error would be thrown due to the matcha.config.json file not being empty but not containing information relating to a resource group.

This has been tested with both stacks.

One possible improvement is that when using the default stack an empty chroma folder is still created within .matcha/infrastructure/resources.

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)

* 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

* 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

…core-function-for-updating-stack-type # Conflicts: # src/matcha_ml/core/core.py

…dating-stack-type' into RPD-284-update-azurerunner-to-select-terraform-files

…update-azurerunner-to-select-terraform-files

KirsoppJ

LGTM

JonoCX

Nice work! Just one comment about separating functionality

src/matcha_ml/core/core.py

JonoCX

Nice, a couple of architectural changes and we're good to go

src/matcha_ml/config/matcha_config.py

JonoCX

LGTM. It'd be good to address the nit to simplify the code a little

JonoCX · 2023-08-11T09:19:08Z

src/matcha_ml/core/core.py

    with remote_state_manager.use_lock(), remote_state_manager.use_remote_state():
        project_directory = os.getcwd()
        destination = os.path.join(
            project_directory, ".matcha", "infrastructure", "resources"
        )
+
+        stack = MatchaConfigService.get_stack()


Nit: this stack shouldn't be None seeing as you set the stack earlier if there is no stack, so you can simplify this into a one liner on 294: MatchaConfigService.get_stack().value (no call to lower() as that's done in stack_set either way)

Yeah, this was the original implementation but mypy doesn't like this though.

Also, I believe lower() is required as the values are stored in matcha.config.json as LLM or DEFAULT

Perhaps adding a type to the stack variable in the original implementation may help: stack: Optional[MatchaConfigComponentProperty] = ...

On the lower(), if the internals are using the stack enum (they should), then the values should all be lower case

No worries if there's no work around for mypy

Removed lower casing. No work around for mypy.

…update-azurerunner-to-select-terraform-files

JonoCX

✅ 🌯

* [RPD-282] adds stack cli command (#179) * adds tests and logic for stack and stack set cli commands * fixes typos * updated for comments * addresses comments * [RPD-285] Create LLM stack Terraform files (#182) * Add initial LLM Terraform files * Update Terraform Kubernetes Chroma deployment * Update Terraform docs * Revert default variables * Add missing aks module * [RPD-283] add core function for updating stack type (#185) * [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 * enum and metaclass * enum and metaclass * bug removal * docstring stack_set * docstring file exists * fix current tests * add a couple tests, move logic for updating to configservice * lowercase-ify arg to enum * american spelling * american spelling * chris' comment re overwriting * friendlier api * Runtime error if not recognised arg type fpor update() * Runtime error if not recognised arg type fpor update() * update provision to use new update() API * update stack name * tests * tests * tests * quote marks * type version, mypy * various review comments * test for update() function * update tests to include quotes. * fixing tests * tidy tests, test error if resources provisioned * clearer error handling * remove unnecessary context * update test to new error type --------- Co-authored-by: Callum Wells <68609181+swells2020@users.noreply.github.com> * [RPD-286] Add documentation on "stacks" and the new LLM stack (#189) * Initial version of docs * Add motivation to stacks docs * Various docs updates based on reviews * [RPD-284] Update AzureRunner to select the Terraform files corresponding to the stack name in the config file (#187) * [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 * enum and metaclass * enum and metaclass * bug removal * docstring stack_set * docstring file exists * fix current tests * add a couple tests, move logic for updating to configservice * lowercase-ify arg to enum * american spelling * american spelling * chris' comment re overwriting * friendlier api * Runtime error if not recognised arg type fpor update() * Runtime error if not recognised arg type fpor update() * update provision to use new update() API * update stack name * tests * tests * tests * Move files for selection * Clean up file * Remove a test that no longer describes the expected functionality * Remove a test that no longer describes the expected functionality * Fix tests * Stop creation of config file in local directory when running tests * Update check to use MatchaConfig object instead of to dict * Add get current stack name function to MatchaConfigService * Update get stack function * Fix test * Remove unnecessary lower casing * Fix tests * updates stack handling * Revert "updates stack handling" This reverts commit b42b31d. * [RPD-289] Update the Chroma Terraform within the LLM stack to use Helm #194 * [RPD-292] [BUG] Update AzureTemplate to not create redundant folders during provisioning (#195) * splits default stack * updates llm stack * [RPD-303] Update stack_set docstring to include example and raises (#196) --------- Co-authored-by: Callum Wells <68609181+swells2020@users.noreply.github.com> Co-authored-by: KirsoppJ <40233184+KirsoppJ@users.noreply.github.com> Co-authored-by: Jonathan Carlton <JonoCX@users.noreply.github.com>

KirsoppJ and others added 25 commits August 3, 2023 15:34

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

b26b9c2

* 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

enum and metaclass

4b30712

Merge remote-tracking branch 'origin/fixes/RPD-278' into RPD-283-add-…

db9a7b7

…core-function-for-updating-stack-type # Conflicts: # src/matcha_ml/core/core.py

enum and metaclass

d491b6f

bug removal

e8faecc

docstring stack_set

d22f311

docstring file exists

ac1b73c

fix current tests

c17071e

add a couple tests, move logic for updating to configservice

5472dc4

lowercase-ify arg to enum

844a20b

american spelling

f3437d7

american spelling

23b7a3d

chris' comment re overwriting

2a5d790

friendlier api

21c1c32

Runtime error if not recognised arg type fpor update()

a064066

Runtime error if not recognised arg type fpor update()

4e5d269

update provision to use new update() API

60a11fa

update stack name

f13356a

tests

6ff719e

tests

db6f806

tests

8a98583

Move files for selection

efc2841

Merge remote-tracking branch 'origin/RPD-283-add-core-function-for-up…

4647dcb

…dating-stack-type' into RPD-284-update-azurerunner-to-select-terraform-files

Clean up file

a66e11f

Christopher-Norman added the enhancement New feature or request label Aug 8, 2023

Christopher-Norman self-assigned this Aug 8, 2023

Christopher-Norman added 3 commits August 8, 2023 10:55

Remove a test that no longer describes the expected functionality

d7624ca

Remove a test that no longer describes the expected functionality

7995331

Fix tests

0427a86

Christopher-Norman added 3 commits August 8, 2023 11:36

Stop creation of config file in local directory when running tests

1799e5d

Update check to use MatchaConfig object instead of to dict

4330ea7

Merge remote-tracking branch 'origin/feature/llm-stack' into RPD-284-…

0b52444

…update-azurerunner-to-select-terraform-files

Christopher-Norman changed the title ~~[WIP] [RPD-284] Update AzureRunner to select the Terraform files corresponding to the stack name in the config file~~ [RPD-284] Update AzureRunner to select the Terraform files corresponding to the stack name in the config file Aug 9, 2023

KirsoppJ approved these changes Aug 10, 2023

View reviewed changes

JonoCX approved these changes Aug 10, 2023

View reviewed changes

src/matcha_ml/core/core.py Outdated Show resolved Hide resolved

Add get current stack name function to MatchaConfigService

fe560b9

JonoCX requested changes Aug 10, 2023

View reviewed changes

src/matcha_ml/config/matcha_config.py Outdated Show resolved Hide resolved

src/matcha_ml/config/matcha_config.py Outdated Show resolved Hide resolved

Christopher-Norman added 2 commits August 11, 2023 09:44

Update get stack function

22d4bdf

Fix test

5b853ae

JonoCX approved these changes Aug 11, 2023

View reviewed changes

Christopher-Norman added 3 commits August 14, 2023 11:20

Remove unnecessary lower casing

5822a17

Merge remote-tracking branch 'origin/feature/llm-stack' into RPD-284-…

9f59992

…update-azurerunner-to-select-terraform-files

Fix tests

43a5367

JonoCX approved these changes Aug 14, 2023

View reviewed changes

Christopher-Norman merged commit 407be1c into feature/llm-stack Aug 14, 2023
2 checks passed

Christopher-Norman deleted the RPD-284-update-azurerunner-to-select-terraform-files branch August 14, 2023 10:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RPD-284] Update AzureRunner to select the Terraform files corresponding to the stack name in the config file #187

[RPD-284] Update AzureRunner to select the Terraform files corresponding to the stack name in the config file #187

Christopher-Norman commented Aug 8, 2023 •

edited

Loading

KirsoppJ left a comment

JonoCX left a comment

JonoCX left a comment

JonoCX left a comment

JonoCX Aug 11, 2023

Christopher-Norman Aug 11, 2023

Christopher-Norman Aug 11, 2023

JonoCX Aug 14, 2023

Christopher-Norman Aug 14, 2023

JonoCX left a comment

[RPD-284] Update AzureRunner to select the Terraform files corresponding to the stack name in the config file #187

[RPD-284] Update AzureRunner to select the Terraform files corresponding to the stack name in the config file #187

Conversation

Christopher-Norman commented Aug 8, 2023 • edited Loading

Checklist

Type of change

KirsoppJ left a comment

Choose a reason for hiding this comment

JonoCX left a comment

Choose a reason for hiding this comment

JonoCX left a comment

Choose a reason for hiding this comment

JonoCX left a comment

Choose a reason for hiding this comment

JonoCX Aug 11, 2023

Choose a reason for hiding this comment

Christopher-Norman Aug 11, 2023

Choose a reason for hiding this comment

Christopher-Norman Aug 11, 2023

Choose a reason for hiding this comment

JonoCX Aug 14, 2023

Choose a reason for hiding this comment

Christopher-Norman Aug 14, 2023

Choose a reason for hiding this comment

JonoCX left a comment

Choose a reason for hiding this comment

Christopher-Norman commented Aug 8, 2023 •

edited

Loading