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

azure: add 'tags' argument to provision_instance #630

Merged
merged 2 commits into from
Oct 24, 2022

Conversation

phi-line
Copy link
Contributor

I found this issue after seeing the latest integration test fail on GH actions:
https://github.com/skyplane-project/skyplane/actions/runs/3293577856


Also adds autoflake to poetry dev deps

Copy link
Contributor

@parasj parasj left a comment

Choose a reason for hiding this comment

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

@phi-line This is fantastic, thanks for submitting this! This does seem to fix the integration test. I can't get Github Actions to pass so I'm investigating ways to support this. Once tests pass, this should be good to merge.

@parasj parasj added the safe to test Label used to trigger CI tests for external forks label Oct 23, 2022
@parasj parasj added safe to test Label used to trigger CI tests for external forks and removed safe to test Label used to trigger CI tests for external forks labels Oct 23, 2022
@parasj
Copy link
Contributor

parasj commented Oct 23, 2022

@phi-line Thanks for the patience. I got it so I can trigger cloud unit tests against forks. Apologies for any spam on your branch. Feel free to merge when ready.

@phi-line
Copy link
Contributor Author

phi-line commented Oct 24, 2022

@parasj thanks for fixing the issue with GH actions when triggered by forks. I noticed that we are using merge commits to update the branches to HEAD at main. It might leave a cleaner history to rebase these changes instead. Otherwise, we get a lot of messages like these:
image
Edit: I removed the merge commits and rebased latest from main

Feel free to merge when ready

I'm ready to merge but I do not have permissions. "Only those with write access to this repository can merge pull requests."

CONTRIBUTING.rst mentions usage of autoflake, but this package was
not included in pyproject.toml
@parasj parasj merged commit 6388037 into skyplane-project:main Oct 24, 2022
@parasj
Copy link
Contributor

parasj commented Oct 24, 2022

@phi-line Thanks, we squash and merge so maintaining clean history is usually not needed. This preserves linear history with original branch state in the PR. I just merged it, good to know that others cannot usually merge.

Thank you for your first contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test Label used to trigger CI tests for external forks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants