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

Update Catalog with skypilot#1204 #2

Merged
merged 2 commits into from
Oct 11, 2022

Conversation

Michaelvll
Copy link
Collaborator

@Michaelvll Michaelvll commented Oct 9, 2022

@Michaelvll Michaelvll changed the title Update Catalog with [skypilot#1204](https://github.com/skypilot-org/skypilot/pull/1204) Update Catalog with skypilot#1204 Oct 9, 2022
@concretevitamin
Copy link
Member

A thought for future: it may be interesting for the fetchers to print out a report to say: N new instance types; on-demand: N prices have changed in M zones; spot: … etc. This can be useful to put in this repository’s readme or put in SkyPilot’s release notes.

@Michaelvll
Copy link
Collaborator Author

The following are the updates in the new catalog, where #resources indicates new (InstanceType, Region, AvailabilityZone) tuples appear in the table

       #resources  #prices  #spot_prices
aws           365      387          4462
azure          67       70          2602
gcp           264      489          1381

@concretevitamin
Copy link
Member

Should this be updating v2 vs. adding a v3? @WoosukKwon

@WoosukKwon
Copy link
Collaborator

WoosukKwon commented Oct 10, 2022

Should this be updating v2 vs. adding a v3?

@concretevitamin I think we should keep it as v2 since there's no schema change.

Copy link
Collaborator

@WoosukKwon WoosukKwon left a comment

Choose a reason for hiding this comment

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

Thanks @Michaelvll for submitting this PR! I've tested some instances and they worked well.

@concretevitamin
Copy link
Member

Since there are some new instance info (InstanceType, Region, AvailabilityZone), does this mean an “older” v2 can’t launch some of these, while a “newer” v2 suddenly can? From a user’s perspective this seems a bit weird.

@WoosukKwon
Copy link
Collaborator

@concretevitamin Oh I just realized that the schema has changed. Considering the discussion we had on the v1 -> v2 update, we should bump up the schema version.

@Michaelvll
Copy link
Collaborator Author

Michaelvll commented Oct 10, 2022

Good point! I just bumped it up to v3. PTAL

Copy link
Collaborator

@WoosukKwon WoosukKwon left a comment

Choose a reason for hiding this comment

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

LGTM! Let's merge this!

@Michaelvll Michaelvll merged commit 76b7d2d into skypilot-org:master Oct 11, 2022
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