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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue: Update dev container configuration #6172

Closed
bamurtaugh opened this issue Jun 14, 2023 · 3 comments 路 Fixed by #6189
Closed

Issue: Update dev container configuration #6172

bamurtaugh opened this issue Jun 14, 2023 · 3 comments 路 Fixed by #6189

Comments

@bamurtaugh
Copy link
Contributor

Issue you'd like to raise.

Hi folks! 馃憢 My name is Brigit, and I'm a PM on the VS Code team working on dev containers and their open spec. Thank you so much for adding a dev container to this repo and langchainjs - these are fantastic scenarios!

As we're actively working improvements to dev containers and their spec, we've made some changes to the best practices we recommend. For instance, we host an updated set of images and templates as part of the spec in the devcontainers org, rather than in vscode-dev-containers. It looks like the image in this repo uses the deprecated vscode-dev-containers image, and perhaps it could leverage the Poetry Feature instead of Poetry installation scripts in the Dockerfile.

I also tried building the dev container in this repo both in the VS Code Dev Containers extension and GitHub Codespaces, and it didn't work for me as-is (I was stopped at container build), so I think this would be a great opportunity to ensure the dev container works well for all potential contributors too.

It looks like langchainjs uses an updated image from the devcontainers org, which is great!

Suggestion:

I'd love to contribute a PR to this repo with an updated dev container (and perhaps with some additional info in the readme or a mini .devcontainer readme), but I wasn't sure the best tests to ensure the repo runs correctly in an updated dev container. Would you be able to share any recommended steps / commands / checks so that I can ensure any dev container updates work well for how folks should be building and running this repo? Info on how you tested and verified the original PR would be a great help too (so I can try the same steps).

Let me know if there's any other info I can provide, and I can also just open a draft a PR if that'd be easiest for discussion. Thanks so much!

cc @vowelparrot and @jj701 as I see your discussion in #4035.

@jj701
Copy link
Contributor

jj701 commented Jun 14, 2023

@bamurtaugh You could start a PR I think.

@vowelparrot
Copy link
Contributor

Hi @bamurtaugh , thanks for raising this issue! And thanks for contributing to LangChain. We definitely want to improve the devcontainer experience when developing with LangChain. The short answer is that we'd welcome any PR implementing your recommended changes. If you tag me I'll review it as soon as possible.

The longer answer is that we are still working on improving the integration testing for many aspects of LangChain, including things like Dev Containers, and still lean heavily on users raising issues such as this one. Apart from spot checks that they build for a couple different scenarios, we will be OK to approve this. We are looking to distribute ownership of key integrations such as this in the future since you and the VSCode team will always have getter knowledge of the best practices for this component than the LangChain core team.

@bamurtaugh
Copy link
Contributor Author

Thank you both!

I've opened a PR here: #6189.

vowelparrot pushed a commit that referenced this issue Jun 16, 2023
Fixes #6172

As described in #6172, I'd
love to help update the dev container in this project.

**Summary of changes:**
- Dev container now builds (the current container in this repo won't
build for me)
- Dockerfile updates
- Update image to our [currently-maintained Python
image](https://github.com/devcontainers/images/tree/main/src/python/.devcontainer)
(`mcr.microsoft.com/devcontainers/python`) rather than the deprecated
image from vscode-dev-containers
- Move Dockerfile to root of repo - in order for `COPY` to work
properly, it needs the files (in this case, `pyproject.toml` and
`poetry.toml`) in the same directory
- devcontainer.json updates
- Removed `customizations` and `remoteUser` since they should be covered
by the updated image in the Dockerfile
     - Update comments
- Update docker-compose.yaml to properly point to updated Dockerfile
- Add a .gitattributes to avoid line ending conversions, which can
result in hundreds of pending changes
([info](https://code.visualstudio.com/docs/devcontainers/tips-and-tricks#_resolving-git-line-ending-issues-in-containers-resulting-in-many-modified-files))
- Add a README in the .devcontainer folder and info on the dev container
in the contributing.md

**Outstanding questions:**
- Is it expected for `poetry install` to take some time? It takes about
30 minutes for this dev container to finish building in a Codespace, but
a user should only have to experience this once. Through some online
investigation, this doesn't seem unusual
- Versions of poetry newer than 1.3.2 failed every time - based on some
of the guidance in contributing.md and other online resources, it seemed
changing poetry versions might be a good solution. 1.3.2 is from Jan
2023

---------

Co-authored-by: bamurtaugh <brmurtau@microsoft.com>
Co-authored-by: Samruddhi Khandale <samruddhikhandale@github.com>
kacperlukawski pushed a commit to kacperlukawski/langchain that referenced this issue Jun 29, 2023
Fixes langchain-ai#6172

As described in langchain-ai#6172, I'd
love to help update the dev container in this project.

**Summary of changes:**
- Dev container now builds (the current container in this repo won't
build for me)
- Dockerfile updates
- Update image to our [currently-maintained Python
image](https://github.com/devcontainers/images/tree/main/src/python/.devcontainer)
(`mcr.microsoft.com/devcontainers/python`) rather than the deprecated
image from vscode-dev-containers
- Move Dockerfile to root of repo - in order for `COPY` to work
properly, it needs the files (in this case, `pyproject.toml` and
`poetry.toml`) in the same directory
- devcontainer.json updates
- Removed `customizations` and `remoteUser` since they should be covered
by the updated image in the Dockerfile
     - Update comments
- Update docker-compose.yaml to properly point to updated Dockerfile
- Add a .gitattributes to avoid line ending conversions, which can
result in hundreds of pending changes
([info](https://code.visualstudio.com/docs/devcontainers/tips-and-tricks#_resolving-git-line-ending-issues-in-containers-resulting-in-many-modified-files))
- Add a README in the .devcontainer folder and info on the dev container
in the contributing.md

**Outstanding questions:**
- Is it expected for `poetry install` to take some time? It takes about
30 minutes for this dev container to finish building in a Codespace, but
a user should only have to experience this once. Through some online
investigation, this doesn't seem unusual
- Versions of poetry newer than 1.3.2 failed every time - based on some
of the guidance in contributing.md and other online resources, it seemed
changing poetry versions might be a good solution. 1.3.2 is from Jan
2023

---------

Co-authored-by: bamurtaugh <brmurtau@microsoft.com>
Co-authored-by: Samruddhi Khandale <samruddhikhandale@github.com>
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 a pull request may close this issue.

3 participants