-
Notifications
You must be signed in to change notification settings - Fork 191
Add script to generate boilerplate code for new backend #2397
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
scripts/add_backend.py
Outdated
loader=jinja2.PackageLoader( | ||
package_name="dstack", | ||
package_path="_internal/core/backends/template", | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nit) _internal/core/backends/template/*.jinja2
is not actually included in the package, so this won't work in the general case. It does work for me when dstack
is installed in editable mode, but this is probably an implementation detail of pip, so I wouldn't rely on it.
I'd suggest using jinja2.FileSystemLoader
instead.
# TODO: Add all supported regions and default regions | ||
REGIONS = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded regions are not needed for most backends, so I wouldn't include this in the template.
I guess they used to be needed for interactive setup, but now they are only needed for backends with custom-built VM images that are not available in all regions. For other backends, hardcoded regions are rather harmful, as they prevent users from using newly added regions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regions don't need to be hardcoded but they need to be validated, and hardcoding them seems to be the best option for most GPU clouds (have a few regions, don't add new regions often, unlikely to have an API to get all the regions).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, feel free to adjust the template on regions.
# If the provider is added to gpuhunt, you'd typically get offers | ||
# using `get_catalog_offers()` and extend them with availability info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nit) Consider adding a sample call to get_catalog_offers
, as contributors can forget to pass important arguments, such as locations
or configurable_disk_size
.
offers = get_catalog_offers(
backend=BackendType.{{ backend_name|upper }},
locations=self.config.regions or None,
requirements=requirements,
configurable_disk_size=..., # TODO: set in case of boot volume size limits
)
Co-authored-by: jvstme <[email protected]>
Part of #2372
The PR adds a
scripts/add_backend.py
script to generate all the necessary files and classes for a new backend and updates the backend guide to use it.