-
Notifications
You must be signed in to change notification settings - Fork 110
fix: Change image for the multi instance replicated queuemanager test #440
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
Conversation
openjdk:17-slim has disappeared
Summary of ChangesHello @kabir, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical build issue by updating the base Docker image used in the multi-instance replicated queue manager test environments. The previous Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request correctly addresses a build failure by replacing the openjdk:17-slim base image, which is no longer available, with mirror.gcr.io/eclipse-temurin:17-jre-jammy. The change is applied consistently across both test application Dockerfiles. My review includes a suggestion to pin the new base image to a specific digest to ensure build reproducibility and enhance security, which is a recommended practice for managing Docker images.
| @@ -1,4 +1,4 @@ | |||
| FROM openjdk:17-slim | |||
| FROM mirror.gcr.io/eclipse-temurin:17-jre-jammy | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For improved build reproducibility and security, it's a best practice to pin the Docker base image to its immutable SHA256 digest. This prevents unexpected changes if the 17-jre-jammy tag is updated in the future.
You can obtain the digest by pulling the image and inspecting it:
docker pull mirror.gcr.io/eclipse-temurin:17-jre-jammydocker inspect mirror.gcr.io/eclipse-temurin:17-jre-jammy(look forRepoDigests)
Then, you can update the FROM line to include the digest, for example:
FROM mirror.gcr.io/eclipse-temurin:17-jre-jammy@sha256:...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignoring since we don't really do this in other places
| @@ -1,4 +1,4 @@ | |||
| FROM openjdk:17-slim | |||
| FROM mirror.gcr.io/eclipse-temurin:17-jre-jammy | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For improved build reproducibility and security, it's a best practice to pin the Docker base image to its immutable SHA256 digest. This prevents unexpected changes if the 17-jre-jammy tag is updated in the future.
You can obtain the digest by pulling the image and inspecting it:
docker pull mirror.gcr.io/eclipse-temurin:17-jre-jammydocker inspect mirror.gcr.io/eclipse-temurin:17-jre-jammy(look forRepoDigests)
Then, you can update the FROM line to include the digest, for example:
FROM mirror.gcr.io/eclipse-temurin:17-jre-jammy@sha256:...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignoring since we don't really do this in other places
a2aproject#284) …t disconnect Backports key fixes from a2a-python PRs a2aproject#440 and a2aproject#472 to resolve event queue lifecycle issues that prevented task resubscription after client disconnect. - Add background task tracking to DefaultRequestHandler with CompletableFuture - Implement asynchronous cleanup when clients disconnect during streaming - Add EventQueue graceful vs immediate close behavior Fixes a2aproject#283 🦕
a2aproject#284) …t disconnect Backports key fixes from a2a-python PRs a2aproject#440 and a2aproject#472 to resolve event queue lifecycle issues that prevented task resubscription after client disconnect. - Add background task tracking to DefaultRequestHandler with CompletableFuture - Implement asynchronous cleanup when clients disconnect during streaming - Add EventQueue graceful vs immediate close behavior Fixes a2aproject#283 🦕
openjdk:17-slim has disappeared
Upstream: #439