Skip to content

fix: Simplify Java Installation in DevContainer - #6012

Open
davem-bis wants to merge 3 commits into
SQLMesh:mainfrom
davem-bis:feature/DRM/simplify-java-installation-in-devcontainer
Open

fix: Simplify Java Installation in DevContainer#6012
davem-bis wants to merge 3 commits into
SQLMesh:mainfrom
davem-bis:feature/DRM/simplify-java-installation-in-devcontainer

Conversation

@davem-bis

@davem-bis davem-bis commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Description

An apparent breaking change in the SDKMAN! API caused the Java DevContainer feature to fail to install.

This change removes that feature and instead installs Java via the msopenjdk-21 package.

An alternate solution would have been to update the DevContainer feature, however this method appeared more robust against future changes.

Test Plan

Dev Container successfully builds and all tests run by make fast-test succeed.

A caveat - the test tests/core/test_context.py::test_python_model_empty_df_raises fails when run in parallel, but passes when run sequentially. I believe this is unrelated to this change and I am fixing in PR #6013.

Checklist

  • I have run make style and fixed any issues
  • I have added tests for my changes (if applicable) (N/A)
  • All existing tests pass (make fast-test)
  • My commits are signed off (git commit -s) per the DCO

…o a breaking change in the SDKMAN! API causing the previous version to fail to install.

Signed-off-by: davem-bis <68955845+davem-bis@users.noreply.github.com>
@davem-bis
davem-bis marked this pull request as ready for review September 2, 2026 12:30
@cmgoffena13
cmgoffena13 self-requested a review September 5, 2026 03:02
Comment thread .devcontainer/post-create-command.sh Outdated
fi

ALL_DEPENDENCIES="libpq-dev netcat-traditional unixodbc-dev default-jdk msodbcsql18"
ALL_DEPENDENCIES="libpq-dev netcat-traditional unixodbc-dev default-jdk msodbcsql18 msopenjdk-21"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I noticed default-jdk is still here, I think we need to remove it right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Aha. Right I can see what has happened here.

  • In the initial setup of the DevContainer I originally used the java feature to install java.
  • I didn't realise at the time but that was adding the Microsoft apt repository which was allowing msodbcsql18 to be installed here without manually adding the repository.
  • I then added the rest of the requisites from .github/scripts/install-prerequisites.sh here, meaning a Java install was added here too.
  • The Java DevContainer feature breaks, hence this PR. I completely missed the installation of default-jdk here, which turns out is sufficient for the dev container.
  • msopenjdk-21 could replace it, but I think staying as consistent to the install-prerequisites.sh script is preferable, so I've the MS one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In summary, had 3 Java installs at one point! I've reduced it to 1.

Signed-off-by: davem-bis <68955845+davem-bis@users.noreply.github.com>
@cmgoffena13

Copy link
Copy Markdown
Collaborator

@davem-bis okay so whats the problem here? We're accidentally installing two Javas so we're removing one?

@davem-bis

Copy link
Copy Markdown
Contributor Author

@cmgoffena13 - version 1.8.1 of the Java DevContainer feature appears to be broken, the API body it gets back from SDKMAN looks to have changed. This means that the dev container cannot build as is.

Version 1.8.2 works, but installing it as a dependency within the post create script looked to be more robust than relying on an api that can change shape.

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