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.

Sign up for GitHub

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

Jump to bottom

Add test fixture for k8s docker cluster #1185

Merged
merged 1 commit into from
Aug 29, 2024
Merged

Add test fixture for k8s docker cluster #1185

merged 1 commit into from
Aug 29, 2024

Conversation

carolineechen
Copy link
Collaborator

No description provided.

This was referenced Aug 22, 2024
Merged
Merged
Merged
Merged
Merged
Merged
@carolineechen Graphite App
Copy link
Collaborator Author

carolineechen commented Aug 22, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @carolineechen and the rest of your teammates on Graphite Graphite

@carolineechen carolineechen force-pushed the cc/k8s-docker-fixture branch 2 times, most recently from 7e95669 to b9edfcb Compare August 22, 2024 20:50
@carolineechen carolineechen mentioned this pull request Aug 23, 2024
Merged
@carolineechen carolineechen marked this pull request as ready for review August 23, 2024 00:39
dongreenberg
dongreenberg reviewed Aug 23, 2024
View reviewed changes
tests/fixtures/on_demand_cluster_fixtures.py Outdated
"provider": "kubernetes",
"instance_type": "CPU:1",
"memory": ".2",
"image_id": "docker:ubuntu:18.04",
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should do a more specific one (e.g. a Ray or SkyPilot one) to make the testing have to accomodate existing Python / package versions already installed?

dongreenberg
dongreenberg reviewed Aug 23, 2024
View reviewed changes
tests/test_resources/test_clusters/test_on_demand_cluster.py Outdated
@@ -115,6 +117,7 @@ def test_restart_does_not_change_config_yaml(self, cluster):
assert config_yaml_content_after_restart == config_yaml_content

@pytest.mark.level("minimal")
@pytest.mark.skip("invalid")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this test broken?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oops did not mean to submit this to the PR, but k8s tests were hanging on this at one point so I skipped it to let the tests finish. but does look like it is failing on main release tests with ModuleNotFoundError: No module named 'tests.conftest'

@carolineechen carolineechen force-pushed the cc/update-rsync branch 2 times, most recently from 599d8e2 to c901231 Compare August 23, 2024 16:18
@carolineechen Graphite App
Copy link
Collaborator Author

carolineechen commented Aug 29, 2024 •
edited
Loading

Merge activity

@carolineechen carolineechen changed the base branch from cc/update-rsync to graphite-base/1185 August 29, 2024 21:35
@carolineechen carolineechen changed the base branch from graphite-base/1185 to main August 29, 2024 21:39
@carolineechen carolineechen force-pushed the cc/k8s-docker-fixture branch 3 times, most recently from a99e2c7 to 38a6ef7 Compare August 29, 2024 21:44
@carolineechen carolineechen merged commit b659124 into main Aug 29, 2024
11 checks passed
@carolineechen carolineechen deleted the cc/k8s-docker-fixture branch August 29, 2024 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dongreenberg dongreenberg dongreenberg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@carolineechen @dongreenberg