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

Fix devcontainer Dockerfile typo (fixes #3114) #3115

Merged
merged 6 commits into from Nov 18, 2023

Conversation

daltonv
Copy link
Contributor

@daltonv daltonv commented Nov 14, 2023

There was a small typo with the name of the requirements.txt file that prevented the whole image from being built.

Fixes #3114

Proposed Changes

  1. Fix the typo in the Dockerfile preventing the build

Readiness Checklist

Author/Contributor

  • Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
  • If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

There was a small typo with the name of the requirements.txt file that
prevented the whole image from being built.
.devcontainer/Dockerfile Outdated Show resolved Hide resolved
@echoix
Copy link
Collaborator

echoix commented Nov 14, 2023

It's funny that you mention this out, since I've been working around this for the last two days on my fork when fiddling with Megalinter. As I'm often available in the evenings, and I never want to open up my full computer so I can sleep, I like using only my cellphone to keep things limited in scope. I recently learned about CodeSandbox, that also uses the devcontainer specification. It fails to start, just like on Codespaces.

However the devcontainer.json was broken even before the change to have a requirements.txt and a makefile to bootstrap in Gitpod.io (that I also use when on my computer for Megalinter).

For codespaces specifically, I don't agree that a specialized docker image is the best choice for users. Anything other that their base image is charged even when off (when not deleted).
I'm more of the opinion that these kind of devcontainers should more be a standard clean environment that can't be broken by your own project, so its possible to 1) start over cleanly, and 2) know if it is your config/projectvthat is a problem or not.

I honestly think it's better to use something like the universal image, and use the features section of the devcontainer to add python functionality, and maybe then use the makefile structure (that isn't that bad, the automatic help output and autocompletion make it easy) that was contributed to add missing dependencies. That way, the container is already started and we can figure the rest out.

I have a similar opinion about the Gitpod using a specialized Dockerfile instead of a more common base, and since the python 3.11 is a little bit older than 3.11.4-3.11.5-3.11.6, there is a bug in one of the libraries that has a workaround that isn't working. I manually "fixed" it each time by adding the missing packages to use a ppa, then using this python ppa https://launchpad.net/~deadsnakes/+archive/ubuntu/ppa (but installation doesn't work correctly with the state of the image, I have to use --fix-broken, then retry). Then I can use anything. If pyenv wouldn't have to compile python from source, it would have been able to use the repos' .python-version directly and it would have been great, just like stock Gitpod.

Ok, so in a try here https://github.com/echoix/megalinter/tree/edit/devcontainer, I tried moving the existing devcontainer to a subfolder, and creating another one with less options, so it's possible to select which one with "new with options". image

Also let's not forget that the images changed, and aren't published through vscode, but as devcontainers. Like https://github.com/devcontainers/images/tree/main/src/python
(See the links at the top of https://github.com/microsoft/vscode-dev-containers/tree/main/containers/python-3)

@daltonv
Copy link
Contributor Author

daltonv commented Nov 15, 2023

To be honest I haven't messed with with codespaces at all, so I'm not sure about the billing story there. It would be worth looking into that later though.

I will update this PR with the inconsistent comment removed, and then I think it is ready to merge.

This comment is no longer relvant and will just confure users.
@echoix
Copy link
Collaborator

echoix commented Nov 15, 2023

To be honest I haven't messed with with codespaces at all, so I'm not sure about the billing story there. It would be worth looking into that later though.

I will update this PR with the inconsistent comment removed, and then I think it is ready to merge.

It's alright. It's a great step for now

@nvuillam
Copy link
Member

@echoix are we all good here ? :) if so, plz can u merge ? :)

@echoix echoix merged commit 7420370 into oxsecurity:main Nov 18, 2023
6 checks passed
@echoix
Copy link
Collaborator

echoix commented Nov 18, 2023

Thanks!

@daltonv daltonv deleted the dv_devcontainer_build branch March 27, 2024 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VSCode devcontainer not building
3 participants