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

feat: create environment for every PR using Uffizzi #6672

Closed
wants to merge 1 commit into from

Conversation

daramayis
Copy link

This PR is a part of #6661 and helps demonstrate Uffizzi integration with Notebook.

This will help create preview environments on all of Notebook Pull Requests and help contributors including the maintainers iterate faster on their PRs.
I have created a PR over daramayis#1 (comment) to show what it looks like to have a preview environment deployed against your PR.

You can check out the preview over here as well.

@waveywaves

@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch daramayis/f-notebook/uffizzi

@daramayis
Copy link
Author

Hi @RRosio 👋

Any thoughts on the PR? how is it looking :)

Thank you.

@RRosio
Copy link
Collaborator

RRosio commented Jan 12, 2023

Hi @daramayis thank you for the ping. We are looking to the discussion going on in JupyterLab before moving forward. I'll bring it up in the next Notebook meeting!

@jtpio
Copy link
Member

jtpio commented Jan 19, 2023

Thanks @daramayis for working on this!

As mentioned in #6672 (comment) we discussed this PR yesterday at the notebook weekly meeting: jupyter/notebook-team-compass#21 (comment)

While Uffizzi looks like a great service for reviewing PR, we also discussed that someone within the Jupyter Notebook team would have to take ownership of this configuration and maintain it over time.

We already use both Binder and Gitpod for working on the Notebook project in a web browser. Both have minimal setup and have been working well so far:

Gitpod also supports prebuilds so the dev setup can be done before someone chooses to open a workspace, helping save time: https://www.gitpod.io/docs/configure/projects/prebuilds

It looks like one advantage of using Uffizzi would be to trigger the builds automatically when new commits are pushed or when a PR is opened. This seems to overlap with what Gitpod already offers with prebuilds. Also looking at the diff of the PR the configuration involves using tools such as Docker and nginx, tools that notebook team members will likely be unfamiliar with.

Personally I won't have the bandwidth to maintain this configuration, but happy to let the PR opened longer if other contributors would like to chime in.

@jpthurman
Copy link

Hi @jtpio
Thanks for the feedback on this. I'm one of the founders at Uffizzi and can add some context.

-We also offer an internal build service which would reduce a lot of overhead by eliminating the GHA workflow file from this PR.

-Does your team / contributors use Gitpod regularly for development? For Teams that prefer local development they also tend to prefer our preview service because it's purpose designed for PR reviews and operates in a headless fashion. There's no manual steps required at any phase and unless you need logs you don't even need to interact with our UI. When someone wants to review a PR the url is right there ready to go.

-There's no way to remove docker from this PR and no immediate way to remove nginx (on our roadmap we have upgrades that would eliminate this config). Ofc you can always reach out to us for assistance with this set-up if issues arise.

If this changes your perspective please let us know and we can adjust the PR to use our internal build service. Otherwise we can close this out.

Thanks for your consideration

@daramayis
Copy link
Author

Hi @jtpio,

Nothing was heard, so closing the PR.

@daramayis daramayis closed this Feb 1, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants