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

Add devcontainer #15909

Merged
Merged

Conversation

fcollonval
Copy link
Member

References

Add dev container set up to ease development and on boarding.

Code changes

Add devcontainer definition and tune the docker set up to share it.

User-facing changes

None

Backwards-incompatible changes

None

Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@fcollonval
Copy link
Member Author

@trungleduc do you mind reviewing this? Especially to check I did not break the docker dev workflow?

@trungleduc trungleduc self-requested a review March 4, 2024 11:43
@jtpio
Copy link
Member

jtpio commented Mar 4, 2024

Maybe this will also help address some comments from this discussion: #13049

@jtpio
Copy link
Member

jtpio commented Mar 4, 2024

not break the docker dev workflow?

Maybe there is a way to put this under test on CI?

@trungleduc
Copy link
Member

trungleduc commented Mar 4, 2024

not break the docker dev workflow?

Maybe there is a way to put this under test on CI?

the docker setup is tested https://github.com/jupyterlab/jupyterlab/actions/runs/8139526094

@trungleduc
Copy link
Member

Thanks @fcollonval

@fcollonval fcollonval requested a review from jtpio March 13, 2024 14:34
@jtpio
Copy link
Member

jtpio commented Mar 13, 2024

Thanks!

Looks like it needs a lint pass:

image

Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

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

Thanks!


// Use 'postCreateCommand' to run commands after the container is created.
// Populate the yarn cache
"postCreateCommand": "micromamba run pip install -e .",
Copy link
Member

Choose a reason for hiding this comment

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

Maybe with the dev extra?

Suggested change
"postCreateCommand": "micromamba run pip install -e .",
"postCreateCommand": "micromamba run pip install -e \".[dev]\"",

Copy link
Member Author

Choose a reason for hiding this comment

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

That step is needed only to build the JavaScript assets. All python dependencies are stored in a dedicated layer:

# Install all python dependencies only with pip to maximize using its cache
RUN --mount=type=cache,target=/jupyterlab/pip-cache,uid=${NEW_MAMBA_USER_ID},gid=${NEW_MAMBA_USER_GID} \
    PIP_CACHE_DIR="/jupyterlab/pip-cache" SKIP_JUPYTER_BUILDER=1 micromamba run python -m pip install -e  ".[dev,docs,test]" && \
    cd / && \
    rm -rf /home/$MAMBA_USER/jupyterlab_cache

So no need for the extra there.

@jtpio
Copy link
Member

jtpio commented Mar 18, 2024

Opening a Codespace from this PR seems to be working fine. Although the pip install -e . step takes quite a bit of time, but this is likely because the specs for the default (free) Codespace are quite low.

image

@fcollonval
Copy link
Member Author

Opening a Codespace from this PR seems to be working fine. Although the pip install -e . step takes quite a bit of time, but this is likely because the specs for the default (free) Codespace are quite low.

I had the same impression. In the follow-up #15916, I plan to store the base docker image on the GitHub registry, in order to speed up the docker build through cache reuse (option build.cacheFrom).

@fcollonval fcollonval merged commit 2267e0f into jupyterlab:main Mar 25, 2024
78 of 80 checks passed
@fcollonval fcollonval deleted the maintenance/set-up-devcontainer branch March 25, 2024 08:43
@fcollonval
Copy link
Member Author

@meeseeksdev please backport to 4.1.x

@fcollonval
Copy link
Member Author

@meeseeksdev please backport to 4.0.x

meeseeksmachine pushed a commit to meeseeksmachine/jupyterlab that referenced this pull request Mar 25, 2024
Copy link

lumberbot-app bot commented Mar 25, 2024

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout 4.0.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 2267e0faf9ac12052815550084d3bbf2a7a545ca
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #15909: Add devcontainer'
  1. Push to a named branch:
git push YOURFORK 4.0.x:auto-backport-of-pr-15909-on-4.0.x
  1. Create a PR against branch 4.0.x, I would have named this PR:

"Backport PR #15909 on branch 4.0.x (Add devcontainer)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

fcollonval added a commit to fcollonval/jupyterlab that referenced this pull request Mar 25, 2024
jtpio pushed a commit that referenced this pull request Mar 25, 2024
Co-authored-by: Frédéric Collonval <frederic.collonval@webscit.com>
krassowski pushed a commit that referenced this pull request Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants