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

updates vars and secrets in ci #277

Merged
merged 2 commits into from
Nov 17, 2023
Merged

updates vars and secrets in ci #277

merged 2 commits into from
Nov 17, 2023

Conversation

@@ -109,7 +109,7 @@ jobs:
- name: Train
env:
REPO_TOKEN: ${{ secrets.PERSONAL_ACCESS_TOKEN }}
STUDIO_TOKEN: ${{ secrets.STUDIO_TOKEN }}
DVC_STUDIO_TOKEN: ${{ secrets.DVC_STUDIO_TOKEN }}
Copy link
Author

Choose a reason for hiding this comment

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

We were using a mix of STUDIO_TOKEN and DVC_STUDIO_TOKEN before. Now it should consistently use DVC_STUDIO_TOKEN everywhere.

@tibor-mach Minor, but maybe we should use the env var in the deployment example to match here and because it seems a bit more straightforward for the CI case (no real need to configure it globally for one-time use).

@@ -26,6 +26,7 @@ jobs:
deploy-model:
needs: parse
if: "${{ needs.parse.outputs.event == 'assignment' }}"
environment: cloud
Copy link
Author

Choose a reason for hiding this comment

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

In #271, we changed the role from AWS_SAGEMAKER_ROLE to AWS_SANDBOX_ROLE and dropped environment: cloud. I'm taking a guess here that the AWS_SAGEMAKER_ROLE exists and has the correct value and was failing because it's only scoped to the cloud environment, but I need @shcheklein to confirm.

@dberenbaum dberenbaum merged commit cb6b10a into master Nov 17, 2023
2 checks passed
@dberenbaum dberenbaum deleted the update-secrets branch November 17, 2023 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants