-
-
Notifications
You must be signed in to change notification settings - Fork 16.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
Add a devcontainer configuration #4969
Add a devcontainer configuration #4969
Conversation
for more information, see https://pre-commit.ci
…emisargent/flask into add-devcontainer-for-codespaces
4d33107
to
a927a24
Compare
f378540
to
2169e81
Compare
b0e051f
to
a42fe15
Compare
@davidism Thanks in advance for the review! If you have any questions, I'll be happy to answer on Monday. |
…emisargent/flask into add-devcontainer-for-codespaces
Moving back to drafts while I make some other changes. |
The virtualenv is not activated in the terminal that is opened when I create the dev container. As I was switched away typing this, it looks like another process kicked in and started |
Out of curiosity, is the feedback from working on this going back to the Codespaces team as issues to improve the Python integration? |
Is there a way to just not open a terminal or show a welcome message? Or to show a "wait, still setting up" modal, preventing interaction until the setup is complete? |
How can I get a codespace without VSCode installing every extension I have locally as well? Most of those are useless for this devcontainer, and are slowing down the setup process. |
It looks like Codespaces is not following the https://containers.dev spec, it does not honor If I wait a really long time, eventually Codespaces decides to pause installing all my local extensions and runs I'm excited for Codespaces as a way to onboard contributors, but if this is the experience of just getting a basic Python environment working (Flask is really basic compared to most projects), I'm not confident I'm going to be able to maintain this, and I'm worried I'll get more user confusion. Also, if I open this project locally in VSCode and choose "Dev Containers: Reopen in Container", I do not get the same behavior, everything just works. The project UI is hidden, a terminal opens showing the progress of |
Thanks for the feedback - I will be sharing this with the team. Part of my goal in initiating this PR is to identify friction points. Another part of my goal is for me to gain a better understanding of devcontainers in general, so I appreciate your patience in this respect.
This is a question I have as well, and will bring to my team.
As mentioned here, the venv will activate in any new terminal. This is why I added the venv activation step to the custom welcome text, and added the
Because this is a simple project with few dependencies, I removed the I hope that by putting in the effort to find the right configuration for Flask, we can more easily add codespaces support for the other Pallets projects! |
Quick followup - there is not currently a way to prevent specific extensions from being installed. However, this feature has been requested previously and is on the radar for the decontainers organization. See devcontainers/features#386 for additional info. |
I made a few more tweaks to clean this up. Since we don't currently have a workaround for the extension loading issue, I also reenabled the Let me know if you have any further questions/suggestions, but I think this PR is ready to go! Thanks again for the great feedback. |
.devcontainer/on-create-command.sh
Outdated
#!/bin/bash | ||
set -e | ||
|
||
if ! git remote | grep -q "fork"; then |
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.
Since the guide instructs contributors to fork first, is this necessary? When I open a codespace on this PR, I see your fork as origin
, and pallets as upstream
. Maybe we just change the guide, probably a bit simpler to push to origin
than to remember to push to fork
.
Additionally, is there any way to automate creating the fork so that users can start the codespace from pallets? I think the basic GitHub editor does this.
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.
Good catch - I've removed this section and added instructions for automatic forking. If starting from pallets, users will be prompted to create a fork the first time they make a commit. Reference docs are here.
Looking much better now, thanks. Let me know if there are specific issues I should subscribe to regarding the different things that have come up. |
I'm about to merge this, I think it's in a good enough place for contributors at this point. I'm going to run a sprint at PyCon two weeks from now, so I'll get more feedback then. @emisargent have you reported any feedback so far to GitHub? Where can I follow discussions/issues? And where should I provide more feedback in the future? Here's a summary of the issues I'd like to see improved:
|
I was going to enable prebuilds, since it sounded like that might speed up the virtualenv issue, but it looks like that's disabled for the pallets organization. Is it possible to get that enabled? |
The prebuilds if run frequently can add up cost-wise quickly, so picking a low frequency trigger is helpful for budget constraints :) |
This change adds the
.devcontainer
directory with adevcontainer.json
,post-create-command.sh
, andwelcome-message.txt
. The devcontainer configuration automatically runs the initial setup commands when a user creates a codespace from their forked repository, making it easier for users to get setup and start contributing.I opted to exclude virtual environment setup since a codespace is already an isolated, dedicated development environment, and users will only be able to access a single project (i.e., Flask) per codespace.
You can test these changes in a codespace by clicking the
Code
dropdown in this PR.In VS Code, open the Command Palette from the settings gear icon and run the "Codespaces: View Creation Log" command to verify all dependencies were correctly installed.
Checklist:
Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.Python tests aren't applicable to these changes
CHANGES.rst
summarizing the change and linking to the issue... versionchanged::
entries in any relevant code docs.pre-commit
hooks and fix any issues.pytest
andtox
, no tests failed.