-
Notifications
You must be signed in to change notification settings - Fork 201
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
Add Lightning Studio Launch Button #851
base: master
Are you sure you want to change the base?
Conversation
@agoose77 sorry to ping you, but wondering if might have time to review or give your opinion about the idea? |
@chrisjsewell @choldgraf sorry to ping you as well, but was hoping to get a response :) |
@jangenoe sorry to ping you you as well, but since you have been active on this PR before, I thought I would try :) |
@nilsleh thanks for your patience here! Let me just run this past the other maintainers to make sure there aren't any policies about which products we support in our config! |
@agoose77 sorry for the ping abuse, but wondering if there has been already a decision on this? |
@agoose77 sorry to ping again, we would like to include this in the next release that we are planning. |
if provider.lower() != "github": | ||
SPHINX_LOGGER.warning(f"Provider {provider} not supported on Lightning.") | ||
else: | ||
github_path = f"%2F{org}%2F{repo}%2Fblob%2F{branch}%2F{path_rel_repo}" |
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.
I think we probably need to URL escape these.
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.
@agoose77 is the latest commit what you were looking for? At least the tests seem to pass again. Let me know if I need to change something else.
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.
Hi @nilsleh not quite -- I'm worried about the case where org
, repo
, branch
, or path_rel_repo
aren't URL escaped properly.
Equally, it's something that the other launchers would need to worry about, so I'll just merge for now!
This reverts commit e232aff.
To get this over the line, we need to update the snapshot tests. |
Is there something we (@adamjstewart and I) can do to help move this forward? |
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.
I think this is the issue, you should be able to run these tests locally and verify that there are no other issues
Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>
Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>
Closes #850
This PR adds a launch button for Lightning Studios a quiet powerful development platform for Deep Learning applications with GPU support.
As stated in the issue, we would like to make this option available to more easily run tutorials for our Deep Learning Uncertainty library, where some require GPUs, and I would imagine that it could be a quiet useful tool for other documentations.