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 default devcontainer.json configuration #37

Merged
merged 12 commits into from
Jul 26, 2024

Conversation

jordan-gillard
Copy link
Contributor

@jordan-gillard jordan-gillard commented Jul 14, 2024

This PR adds a default devcontainer.json file that includes all currently supported Python interpreters for the project. It also includes recommended VSCode extensions for Python development. This will make developing easier since it unifies developer environments & makes running tox against all supported versions of Python a breeze.

Development containers is an open standard for specifying dev environments. This is an IDE-agnostic way to unify development environments for users who use VSCode, PyCharm, and any other IDE. Users can choose to use the development container. These changes make no difference to users who want to continue with their normal local workflow.

Relates to: #11 - This makes contributing (& thus writing tests) much easier.

@jordan-gillard jordan-gillard changed the title Add default GitHub Codespaces configuration Add default .devcontainer configuration Jul 14, 2024
@jordan-gillard jordan-gillard changed the title Add default .devcontainer configuration Add default devcontainer.json configuration Jul 14, 2024
@GalenReich
Copy link
Collaborator

I like this idea - it seems like it could be a neat (and optional) way of creating a more consistent development environment!

My only concern is that we end up supporting the use of development tools rather than the tool iteself. But I think this is worth it on balance - primarily because it is optional and a user can choose to totally ignore it! Though we should make sure to document its usage somewhere.

When trying out the PR I ran into some issues, I use VSCode which autodetected the dev container config and offered to build and use it ✅ After building, the poetry install command failed saying that python didn't exist ❌ :

Running the postCreateCommand from devcontainer.json...

[21036 ms] Start: Run in container: /bin/sh -c poetry install

[Errno 2] No such file or directory: 'python'
[22250 ms] postCreateCommand failed with exit code 1. Skipping any further user-provided commands.
Done. Press any key to close the terminal.

Looking into this further it looks like this error originates because I 'Reopened my existing folder' in the container (the default option) rather than 'Cloning in the container' which worked without issue. I still don't understand the root cause of the issue - but it is a good demonstration of what can go wrong if you don't know what you're doing! 😅

@jordan-gillard
Copy link
Contributor Author

@GalenReich Sorry about that! The issue looks like it comes from poetry looking for a default Python interpreter but it couldn't find one. Commit baf038d fixes that so that running python or python3 on the command line always links to the python 3.12 interpreter binary. I'm not sure why this is an issue when mounting vs cloning the files into the remote. I suspect it has to do with how the file system was interpreting the container's permissions to search it for the binary (even though it was installed on the container beforehand). It should work now. I was able to check this behavior locally with PyCharm.

My only concern is that we end up supporting the use of development tools rather than the tool itself.

My intention with this is that the dev container will only aid users in contributing to EDGAR. I'm think about this comment #11 (comment) where the user couldn't configure the project because they were unfamiliar with poetry. With a devcontainer they just click on the Code dropdown on the repo's main page or a PR and they can immediately open up the code in an IDE and get started.
image
It takes the hassle away from setting things up. Also, once it works it requires practically zero maintenance. The only time we'll revisit it would be to add a new python interpreter or remove an old one once it's deprecated.

Though we should make sure to document its usage somewhere.

Agreed. I updated the README in commit 9956ef0

@GalenReich
Copy link
Collaborator

With a devcontainer they just click on the Code dropdown on the repo's main page or a PR and they can immediately open up the code in an IDE and get started. It takes the hassle away from setting things up. Also, once it works it requires practically zero maintenance. The only time we'll revisit it would be to add a new python interpreter or remove an old one once it's deprecated.

Holy moly, that is very cool!

Copy link
Collaborator

@GalenReich GalenReich left a comment

Choose a reason for hiding this comment

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

This is looking good!

@GalenReich GalenReich merged commit d3b836a into bellingcat:main Jul 26, 2024
4 checks passed
@jordan-gillard jordan-gillard deleted the add-codespaces-config branch July 26, 2024 22:33
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.

2 participants