Cleanup and unify Dockerfiles

[gagb]

Why are these changes needed?

Related issue number

#1286 #1332

Checks

• I've included any doc changes needed for https://microsoft.github.io/autogen/. See https://microsoft.github.io/autogen/docs/Contribute#documentation to build and test documentation locally.
• I've added tests (if relevant) corresponding to the changes introduced in this PR.
• I've made sure all auto checks have passed.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d243db7) 32.48% compared to head (8defeaf) 32.48%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1333   +/-   ##
=======================================
  Coverage   32.48%   32.48%           
=======================================
  Files          41       41           
  Lines        4907     4907           
  Branches     1120     1120           
=======================================
  Hits         1594     1594           
  Misses       3187     3187           
  Partials      126      126           

Flag	Coverage Δ
unittests	32.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[qingyun-wu]

Nice PR! I believe there are more changes that need to be made:

1. Since the .devcontainer folder is a hidden directory, the added readme file in this directory is not that visible. I think we either need to add a pointer to this added readme file somewhere in the documentation website or move the file there.
2. Because of the move of those docker files, some of the command lines become invalid immediately; we will need to change that accordingly. (I can help with it in this PR).

[gagb]

Nice PR! I believe there are more changes that need to be made:

1. Since the .devcontainer folder is a hidden directory, the added readme file in this directory is not that visible. I think we either need to add a pointer to this added readme file somewhere in the documentation website or move the file there.

We could add a pointer here: https://microsoft.github.io/autogen/docs/Contribute
And also address the last task in #1286 with it?

[davorrunje]

Dockerfiles under .devcontainer/base, .devcontainer/dev and .devcontainer/full are based on python:3.11-slim-bookworm, while under .devcontainer and .devcontainer/studio under mcr.microsoft.com/vscode/devcontainers/python:3.10.

Why different base Dockerfiles?

Why not use mcr.microsoft.com/vscode/devcontainers/* based images that are optimized for usage with VSCode (see under https://github.com/microsoft/vscode-dev-containers/tree/main/containers/python-3)?

[qingyun-wu]

Dockerfiles under .devcontainer/base, .devcontainer/dev and .devcontainer/full are based on python:3.11-slim-bookworm, while under .devcontainer and .devcontainer/studio under mcr.microsoft.com/vscode/devcontainers/python:3.10.

Why different base Dockerfiles?

Why not use mcr.microsoft.com/vscode/devcontainers/* based images that are optimized for usage with VSCode (see under https://github.com/microsoft/vscode-dev-containers/tree/main/containers/python-3)?

There is no specific reason. They are added by different people. I believe we can unify these.

For base images, I don't have a good sense of which one is better. If the ones optimized for VS code are preferred, I am fine switching to those. @davorrunje do you suggest changing all the base images to a VS code optimized one, .e.g., mcr.microsoft.com/devcontainers/python:3.11 or just some certain ones of them, e.g.,the one in .devcontainer/dev?

[qingyun-wu]

@gagb revised relevant documentation needed because of the cleanup. For the base image, Davor's suggestion for using the VS code-optimized one sounds good to me. But I haven't changed that. I tried mcr.microsoft.com/devcontainers/python:3.11. Working well so far.

[davorrunje]

Dockerfiles under .devcontainer/base, .devcontainer/dev and .devcontainer/full are based on python:3.11-slim-bookworm, while under .devcontainer and .devcontainer/studio under mcr.microsoft.com/vscode/devcontainers/python:3.10.
Why different base Dockerfiles?
Why not use mcr.microsoft.com/vscode/devcontainers/* based images that are optimized for usage with VSCode (see under https://github.com/microsoft/vscode-dev-containers/tree/main/containers/python-3)?

There is no specific reason. They are added by different people. I believe we can unify these.

For base images, I don't have a good sense of which one is better. If the ones optimized for VS code are preferred, I am fine switching to those. @davorrunje do you suggest changing all the base images to a VS code optimized one, .e.g., mcr.microsoft.com/devcontainers/python:3.11 or just some certain ones of them, e.g.,the one in .devcontainer/dev?

It is probably not worth the effort trying to get a custom Dockerfile to work well both locally and in Codespaces, it is safer to use mcr.microsoft.com/devcontainers/python:3.* and build on top of it. I quickly looked into what they did and that's just too much to figure out. Codespaces are important for code reviews.

[gagb]

@gagb revised relevant documentation needed because of the cleanup. For the base image, Davor's suggestion for using the VS code-optimized one sounds good to me. But I haven't changed that. I tried mcr.microsoft.com/devcontainers/python:3.11. Working well so far.

What's the reason to shift to 3.11 image when current is using 3.10?

Also I locally built our primary base Dockerfile i.e., .devcontainer/Dockerfile

docker build -f .devcontainer/Dockerfile -t autogen_main .
docker run -it -p 8081:3000 -v `pwd`:/autogen autogen_main bash
# inside the container
cd /autogen
sudo pip install -e .  # without sudo I get an error installing (see below)

Given that this works, what is the utility of .devcontainer/base/.
This example shows that our primary base is compatible with both vscode, Codespaces, and local dev.
I am not an expert so please double check my arguments. (cc @qingyun-wu, @davorrunje)

Error if sudo isn't specified.

ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
databind-json 4.4.2 requires typing-extensions<4.7,>=3.10.0, but you have typing-extensions 4.9.0 which is incompatible.
databind-core 4.4.2 requires typing-extensions<4.7,>=3.10.0, but you have typing-extensions 4.9.0 which is incompatible.

My suggestion:

• delete .devcontainer/base and only provide primary base. Update .devcontainer/README.md
• merge and create a separate issue that updates dev and full to use a different base image
• double-check instructions in Contribute.md

[gagb]

I removed .devcontainer/base
dev and full can be updated to use different base images in a separate PR -- this change isn't blocking anyone.

[davorrunje]

I am not sure about moving additional directories under .devcontainer. As far as I understand, and I don't know much about it, VS Code uses .devcontainer/devcontainer.json to start a docker container specified in it when you run it. That's actually how I use it and it works very well. I don't really know how to start VS Code with alternative Docker images without changing .devcontainer/devcontainer.json, which is a file tracked by git and should be used for local configuration. Of course, it makes sense to use other containers in different circumstances, but maybe .devcontainer is not a good place for them.

[gagb]

I am not sure about moving additional directories under .devcontainer. As far as I understand, and I don't know much about it, VS Code uses .devcontainer/devcontainer.json to start a docker container specified in it when you run it. That's actually how I use it and it works very well. I don't really know how to start VS Code with alternative Docker images without changing .devcontainer/devcontainer.json, which is a file tracked by git and should be used for local configuration. Of course, it makes sense to use other containers in different circumstances, but maybe .devcontainer is not a good place for them.

@davorrunje please see GitHub's documentation for why subdirectories: https://docs.github.com/en/codespaces/setting-up-your-project-for-codespaces/adding-a-dev-container-configuration/introduction-to-dev-containers#devcontainerjson

Screenshot

How to use alt container

See reply to Chi here: #1241 (comment)

(This is already in the documentation in this PR in .devcontainer/README.md)

[davorrunje]

How to use alt container

See reply to Chi here: #1241 (comment)

(This is already in the documentation in this PR in .devcontainer/README.md)

Thanks for the explanations 👍