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

Update dev container #6189

Merged
merged 10 commits into from
Jun 16, 2023

Conversation

bamurtaugh
Copy link
Contributor

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 (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)
  • 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

Who can review?

Tag maintainers/contributors who might be interested: @vowelparrot

I was able to run through the various make commands mentioned in contributing.md successfully with this updated setup, and please let me know if there are questions or other tests I can help with to ensure everything is running well. Thanks so much!

.devcontainer/README.md Show resolved Hide resolved
.devcontainer/README.md Outdated Show resolved Hide resolved
.devcontainer/README.md Show resolved Hide resolved
.devcontainer/devcontainer.json Outdated Show resolved Hide resolved
.devcontainer/devcontainer.json Outdated Show resolved Hide resolved
.devcontainer/docker-compose.yaml Outdated Show resolved Hide resolved

# Use the Python base image
ARG VARIANT="3.11-bullseye"
FROM mcr.microsoft.com/vscode/devcontainers/python:0-${VARIANT} AS langchain-dev-base
FROM mcr.microsoft.com/devcontainers/python:0-${VARIANT} AS langchain-dev-base
Copy link

@samruddhikhandale samruddhikhandale Jun 15, 2023

Choose a reason for hiding this comment

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

FYI, we will soon release a new major version of python in order to support debian:bookworm
devcontainers/images#622

I'd recommend switching once it's live to continue receiving security patches

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for letting us know, @samruddhikhandale !

@vowelparrot
Copy link
Contributor

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

cc @eyurtsev FYI

@vowelparrot
Copy link
Contributor

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

Are there any logs you would be able to share on this one? It's surprising that it would take 30 minutes unless we're installing ALL the optional dependencies. The list of actual requirements isn't that large

@vowelparrot
Copy link
Contributor

What's the fastest way to build the dev container and codespace on your branch? I can look into it tomorrow morning if I get to it before you see this.

bamurtaugh and others added 4 commits June 14, 2023 22:09
Co-authored-by: Samruddhi Khandale <samruddhikhandale@github.com>
Co-authored-by: Samruddhi Khandale <samruddhikhandale@github.com>
Co-authored-by: Samruddhi Khandale <samruddhikhandale@github.com>
Co-authored-by: Samruddhi Khandale <samruddhikhandale@github.com>
@bamurtaugh
Copy link
Contributor Author

Thanks so much for the review @samruddhikhandale!

@vowelparrot you should be able to use the Code dropdown at the top of this PR:

image

Or from my fork: https://github.com/bamurtaugh/langchain/tree/bamurtaugh/dev-container.

It'd be awesome to have you try it out and see if there's something apparent from your perspective that'd be causing it to hang.

@vowelparrot
Copy link
Contributor

We probably don't need to install all of "--with dev,test,docs" on container creation (unless it's really terrible to add additional dependencies). I'd remove the "docs" group and see how long it takes then

@bamurtaugh
Copy link
Contributor Author

bamurtaugh commented Jun 15, 2023

I'd remove the "docs" group and see how long it takes then

Thanks for the recommendation @vowelparrot! I'm trying that now. Still going and it's been about 8 minutes, so still feels longer than expected?

Edit: Have to step away for a bit, but it's still building at the 20-minute mark. 🤔 Does it take this long for other folks to generally run a poetry install?

@vowelparrot vowelparrot merged commit ccd916b into langchain-ai:master Jun 16, 2023
This was referenced Jun 25, 2023
@lynnhio lynnhio mentioned this pull request Jul 24, 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.

Issue: Update dev container configuration
4 participants