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

Added Dev Container support #474

Merged
merged 6 commits into from
May 21, 2023

Conversation

acbgbca
Copy link
Contributor

@acbgbca acbgbca commented May 20, 2023

For #473

This adds the necessary files to support developing with Dev Containers. It should have no impact on anyone who chooses to develop without Dev Containers, it simply adds it as an option.

The changes:

  • devcontainer.json - The Dev Container configuration. Uses an image with Python 3.10 and NodeJS support added. It also automatically runs several of the necessary commands on creation making it easier for new users
  • .gitattributes - Dev Containers appear to have this issue on Windows where the files are checked out with Windows file endings, then loaded into a Linux dev container where it expects different line endings. This resolves that. It should have no impact on anyone else
  • readme.md - Added that Dev Containers are supported, and added a link which would automatically check the code out in a Dev Container. Note that the link will not work until the change has been merged due to the repository currently not having the necessary files.

If you have any questions on how this works let me know and I can add further detail. I'd be happy to more detail to the documentation around this.

Copy link
Owner

@sissbruecker sissbruecker left a comment

Choose a reason for hiding this comment

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

Thanks, seems to work fine. New files should be added to .dockerignore.

.devcontainer/devcontainer.json Show resolved Hide resolved
.gitattributes Outdated Show resolved Hide resolved
"forwardPorts": [8000],

// Use 'postCreateCommand' to run commands after the container is created.
"postCreateCommand": "pip3 install --user -r requirements.txt && npm install && mkdir -p data && python3 manage.py migrate",
Copy link
Owner

Choose a reason for hiding this comment

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

linkding also has a few integration tests using Playwright. Playwright needs to additionally download a browser, currently I'm using playwright install chromium in Github Actions. Doing that in the dev container works, but then the tests fail:

python manage.py test

...

╔══════════════════════════════════════════════════════╗
║ Host system is missing dependencies to run browsers. ║
║ Missing libraries:                                   ║
║     libatk-1.0.so.0                                  ║
║     libatk-bridge-2.0.so.0                           ║
║     libcups.so.2                                     ║
║     libdbus-1.so.3                                   ║
║     libatspi.so.0                                    ║
║     libXcomposite.so.1                               ║
║     libXdamage.so.1                                  ║
║     libXfixes.so.3                                   ║
║     libXrandr.so.2                                   ║
║     libgbm.so.1                                      ║
║     libdrm.so.2                                      ║
║     libxkbcommon.so.0                                ║
║     libpango-1.0.so.0                                ║
║     libcairo.so.2                                    ║
║     libasound.so.2                                   ║
╚══════════════════════════════════════════════════════╝

Not a blocker, the integration test setup currently also has other issues like needing to run certain commands beforehand. But maybe something to look into in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added "playwright install chromium && playwright install-deps" to the post create script. The tests run now, however there were 2 failures and 1 error. I don't know whether those are existing problems or whether they are to do with running in the dev container.

If there are other commands that need to be run prior to the tests let me know and I can try adding them in. If post create isn't appropriate there are also some other hooks where scripts can be run.

"forwardPorts": [8000],

// Use 'postCreateCommand' to run commands after the container is created.
"postCreateCommand": "pip3 install --user -r requirements.txt && npm install && mkdir -p data && python3 manage.py migrate && playwright install chromium && playwright install-deps",
Copy link
Owner

Choose a reason for hiding this comment

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

playwright install-deps failed for me:

E: Package 'ttf-ubuntu-font-family' has no installation candidate
E: Unable to locate package libenchant1c2a
E: Unable to locate package libicu66
E: Package 'libjpeg-turbo8' has no installation candidate
Failed to install browser dependencies
Error: Installation process exited with code: 100
[120273 ms] postCreateCommand failed with exit code 1. Skipping any further user-provided commands.

Let's remove the playwright installation for now, I'll figure out how to run the playwright tests separately at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, I wonder why the command works for me and not for you, that shouldn't happen. If it becomes a problem we can use a base Dockerfile for the Dev Container rather then the Microsoft Python Container, that way it can be pre-setup with all dependencies that are required.

For now I have removed the playwright commands. Let me know if there are any other changes required,

@sissbruecker sissbruecker merged commit 417dce7 into sissbruecker:master May 21, 2023
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