-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
RFC: Add dev container configuration #12306
base: main
Are you sure you want to change the base?
Conversation
.devcontainer/devcontainer.json
Outdated
"packages": "mesa-utils,libegl1,^libxcb.*-dev,libx11-xcb-dev,libglu1-mesa-dev,libxrender-dev,libxi-dev,libxkbcommon-dev,libxkbcommon-x11-dev" | ||
}, |
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.
this could probably be trimmed down a little? not sure.
9ef71dd
to
edd8c9f
Compare
The password for VNC access is |
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 like this idea / initiative! I've left mostly questions and a few comments, so I can understand better how the config works.
.devcontainer/devcontainer.json
Outdated
// Zsh plugins | ||
"ghcr.io/devcontainers-contrib/features/zsh-plugins:0": { | ||
"plugins": "zsh-autosuggestions zsh-syntax-highlighting history-substring-search", | ||
"omzPlugins": "https://github.com/zsh-users/zsh-autosuggestions https://github.com/zsh-users/zsh-syntax-highlighting" |
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.
why are these needed? Will the user ever access a terminal in the container (and if so, won't it be a BASH terminal by default?)
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.
Yes, of course :) It's a complete development environment, and if you open a terminal, it will open a "remote" shell inside the Docker container.
The default shell is zsh with oh-my-zsh.
I'm trying to provide a nice and comfy shell experience through these plugins.
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'm trying to provide a nice and comfy shell experience through these plugins.
Another word for "comfy" is "familiar". I don't necessarily object to including zsh
alongside bash
but I'm dubious about making zsh
the default here.
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.
MacOS users have zsh
as the default shell and zsh
is very close to bash
.
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.
yes, I know that since 2019 macOS uses zsh as the default shell. And I've heard that they're similar. What I'm questioning is why tailor the experience in a linux container to feel familiar to macOS users. Sticking with BASH seems like the natural default choice, and deviation from that feels like it needs to be justified.
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.
Both shells are first-class citizens in the MS dev containers. By default, both are shipped, hence I'm shipping both too. The plugins listed here make the zsh experience better and superior to the Bash experience. I have not yet decided which shell shall be the default one. However, I'm surprised this seems to be so controversial. VS Code clearly displays which shell is being used, and when spawning a new terminal, one can select the shell from a dropdown menu. Besides, I don't see how the choice of shell matters as long as one doesn't do shell programming; and this is an MNE dev container, no generic Linux Shell Programming dev container. Like I said, I haven't decided yet which default I think would be best; but It's quite obvious to me though that the zsh prompt looks better and the shell provides more help during interactive use (history substring search being one of those invaluable features; git support being another).
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 have not yet decided which shell shall be the default one.
This is news to me; you originally set zsh
as default (which I noticed at the time), then removed that setting in 7d5c6a2 after my earlier comments. It seems a bit disingenuous to now say "why is this so controversial, they're both included and I haven't chosen a default".
That said, I'm happy if both are included and it's easy (and documented) for users to switch the shell they get in their container. Ideally this would be a one-time config (not something that needs doing every time the container is launched) if that's possible
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.
This is news to me; you originally set
zsh
as default (which I noticed at the time), then removed that setting in 7d5c6a2 after my earlier comments
Ah, I see!! No, this was actually a misunderstanding. The default login shell always was Bash, and the setting you're referring to controls which shell should be spawned by default when creating a new terminal without explicitly selecting a shell. This was in fact an accidental setting that I copied over from another dev container config I'm working with. But this setting never changed the (login) default shell to zsh, nor dropped bash or something :) I had however forgotten to properly set up mamba
and conda
for bash – something I realized and fixed after your comment / question.
That said, I'm happy if both are included and it's easy (and documented) for users to switch the shell they get in their container. Ideally this would be a one-time config (not something that needs doing every time the container is launched) if that's possible
Yep, I'm open for this! for now I'm really still trying to make things work and experimenting a bit! Let's save the heated debates for later! 😁🤗
Oh and, happy holidays to you all!!!
// Configure properties specific to VS Code. | ||
"vscode": { | ||
"settings": { |
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.
seems like fertile ground for bikeshedding. Is there a way for the container to pick up the user's local settings? If not, then can you indicate which of these settings are truly necessary for the container to work, which are not necessary but clearly a good idea (e.g., don't restore port forwards?), and which are your preferences (perhaps "format on type" fits here)?
Thanks for you feedback, @drammock |
would be good to get this "into" 1.7, but i'm not sure if i'll be able to finish it in time i know it's devs-only but it would be nice to have a reference to this in the 1.7 stable docs |
In this case let's not mark it for 1.7 -- let's only mark the things that are blockers for release (which should happen this week ideally). If it gets merged in time though that's great (nothing stopping it if you can get it done)! |
ok I'm fine with that! |
Using |
Thanks for testing, @larsoner The button you're referring to will unfortunately only disable the dialog asking whether you want to open the folder in a dev container; the terminal output from your first screenshot would not be hidden by that. Seems I'll need to think a little more about this… btw how do you like the |
Haven't tried it, on a plane at the moment!
Ahh. That part doesn't bother me too much based on my workflow but I could see it being problematic for others. Maybe at the end of the script there is some way to tell |
Have a safe trip! |
…nces for devs who are not using the Dev Container
|
👩💻 The experience I'm implementing here
Screen.Recording.2023-12-18.at.15.29.08.mov
📚 Background
In the past year, I moved almost all my development work into dev containers.
Until recently, one of the few development workflows I had not yet transitioned to use dev containers was MNE-Python. Reason was that the interactive figures don't work well in a notebook or web browser, and I didn't want to run an X server on my host just to display the figures.
💡 Solution
I created a dev container configuration that deploys a remote resktop feature. It allows for a remote connection via a VNC client, or via the web browser. Browser-based access even works inside VS Code with the built-in Simple Browser.
The dev container runs on Debian 11, installs MNE-python via
conda-forge
, followed by apip
-based editable install of the version in the checked-out repository.🎯 Scope & target audience
🔮 Future plans
sample
andtesting
datasets.🏃♀️ Using the dev container
You need
Open the repository in VS Code and click on Reopen in Container in the bottom right.
The Debian docker image and dev container features including MNE will be downloaded and installed. Lastly, an editable install of MNE will be carried out.
The VNC server will be exposed on
localhost:5901
(you can connect to it with any VNC client, e.g.Screen Sharing.app
on macOS), and the web server can be reached onhttp://localhost:6080
. The password isvscode
.You can now start developing and using MNE-Python. Interactive figures will appear on the screen exposed via the VNC server.
All development dependencies are installed (incl. pre-commit hooks, pytest etc.), and the editor is set to auto-format code via Ruff on saving.
The host SSH agent's socket is transparently forwarded into the container, meaning you should be able to use SSH immediately without requiring any further configuration.
The host's git configuration is copied to the container as well, so your name and email address are set correctly, too.
✋ Limitations
I currently enforce running an AMD64 Debian image on all platforms, including Apple Silicon. The native ARM64 wouldn't work as MNE-Python cannot be installed from
conda-forge
on this platform, as the migration ofaarch64
dependencies is still ongoing. To avoid this, all deps could be potentially installed from PyPI viapip
, but I haven't tried this yet.❓Thoughts?
What do you all think?
cc @cbrnr @britta-wstnr @agramfort @sappelhoff