zenml-io / mlstacks

A series of Terraform based recipes to provision popular MLOps stacks on the cloud.
https://mlstacks.zenml.io/
Apache License 2.0
245 stars 32 forks source link

Close quotes on `kubernetes_context` values in `gcp-modular` #100

Closed cameronraysmith closed 9 months ago

cameronraysmith commented 9 months ago

Failure to close quotations in the stack_file kubernetes_context values leads to errors like

Importing stack 'mlops' into ZenML...
...
ParserError: while parsing a flow mapping
  in "<unicode string>", line 27, column 24:
            configuration: {"kubernetes_context": "gke_zenm ... 
                           ^
expected ',' or '}', but got '<scalar>'
  in "<unicode string>", line 27, column 75:
     ... t": "gke_zenml-cluster-6i6x, "synchronous": True}
                                         ^

when running commands such as

mlstacks deploy -f stack.yaml -d
zenml stack deploy -p gcp -n mlops -r us-east4 -f stack.yaml -d

in zenml v0.44.2 and mlstacks v0.7.5.

strickvl commented 9 months ago

Thanks for catching this @cameronraysmith! We take PRs on the develop branch, so could you possibly rebase this PR / branch on develop instead of making it point to main? https://github.com/zenml-io/zenml/blob/main/CONTRIBUTING.md#-pull-requests-rebase-your-branch-on-develop has instructions in case you need / want that (taken from the sister core zenml repository).

cameronraysmith commented 9 months ago

done.

cameronraysmith commented 9 months ago

Let me know if it needs to be rebased again.

strickvl commented 9 months ago

@cameronraysmith yeah actually not sure what the issue is... the error message seems to be unrelated to the changes + develop branch itself doesn't have issues...

cameronraysmith commented 9 months ago

What is the error message? It's rebased on develop up to https://github.com/zenml-io/mlstacks/commit/71b97b51aab5ee17c261fa3af0cbf6744188fa0e

1b606fe (HEAD -> fix-kubernetes-context-quotation, origin/fix-kubernetes-context-quotation) fix: close quotes on kubernetes_context values
71b97b5 (upstream/develop, develop) Add `skypilot` support for AWS and GCP (#99)
5069e56 update PR template to discourage PRs on main (#101)
64ce5c9 Add remote state deployment + use by default (#97)
strickvl commented 9 months ago

I see what's going on -- here. It's not passing the internal secrets down since you're contributing from a forked branch, so that's why the tests don't work. In any case this is a tiny PR with only fixes, no breaking changes, so I'll merge it into develop once the rest of the tests pass. Thanks again!

cameronraysmith commented 9 months ago

Some of these jobs like aws_test and gcp_test failed in #99 here https://github.com/zenml-io/mlstacks/actions/runs/6310939091 so they aren't going to pass after rebasing on it https://github.com/zenml-io/mlstacks/commit/71b97b51aab5ee17c261fa3af0cbf6744188fa0e. Since you already merged that one there shouldn't be an additional issue with this one.

It's not passing the internal secrets down since you're contributing from a forked branch, so that's why the tests don't work.

I see. You are correct.

cameronraysmith commented 9 months ago

Thank you @strickvl. Apologies for the confusion! It doesn't look like there's an option to allow usage of secrets even when maintainers approve workflow runs.