feat!: Remove hard dependency on MicroProfile Config from the core SDK #469
Conversation
a2aproject#468) Default values will now be used by default. You can supply your own by providing a CDI bean with a higher priority. Also, there is a new 2a-java-sdk-microprofile-config with the previous MicroProfile Config capabilities. If used, this will allow MicroProfile Config configurations of the properties. The reference implementations use this new module Fixes a2aproject#467 🦕
kabir
changed the title
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 significantly refactors the configuration management within the A2A Java SDK. The primary goal is to remove the direct dependency on MicroProfile Config from the core SDK, making it more modular and adaptable to various environments. This is achieved by introducing a new, flexible configuration provider interface and a default implementation that loads properties from classpath resources. An optional integration module is provided for users who still want to leverage MicroProfile Config, ensuring backward compatibility for existing reference implementations while offering greater choice for new integrations. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed refactoring to decouple the core SDK from a hard dependency on MicroProfile Config. The new A2AConfigProvider interface and the DefaultValuesConfigProvider create a flexible and extensible configuration system, which is a great improvement for framework interoperability. The addition of the a2a-java-sdk-microprofile-config module is a thoughtful way to provide backward compatibility for existing users. The changes are well-documented in the READMEs and are accompanied by solid unit tests.
My review comments focus on a consistent maintainability improvement across the newly modified classes: replacing hardcoded configuration key strings with static final constants. This will make the code more robust and easier to maintain.
| @Inject | ||
| @ConfigProperty(name = "a2a.replication.grace-period-seconds", defaultValue = "15") | ||
| A2AConfigProvider configProvider; | ||
|
|
||
| /** | ||
| * Grace period for task finalization in replicated scenarios (seconds). | ||
| * After a task reaches a final state, this is the minimum time to wait before cleanup | ||
| * to allow replicated events to arrive and be processed. | ||
| * <p> | ||
| * Property: {@code a2a.replication.grace-period-seconds}<br> | ||
| * Default: 15<br> | ||
| * Note: Property override requires a configurable {@link A2AConfigProvider} on the classpath. | ||
| */ | ||
| long gracePeriodSeconds; | ||
|
|
||
| @PostConstruct | ||
| void initConfig() { | ||
| gracePeriodSeconds = Long.parseLong(configProvider.getValue("a2a.replication.grace-period-seconds")); | ||
| } |
There was a problem hiding this comment.
To improve maintainability and avoid magic strings, it's best to declare the configuration key as a private static final String constant. This prevents typos and makes it easier to manage keys.
| @Inject | |
| @ConfigProperty(name = "a2a.replication.grace-period-seconds", defaultValue = "15") | |
| A2AConfigProvider configProvider; | |
| /** | |
| * Grace period for task finalization in replicated scenarios (seconds). | |
| * After a task reaches a final state, this is the minimum time to wait before cleanup | |
| * to allow replicated events to arrive and be processed. | |
| * <p> | |
| * Property: {@code a2a.replication.grace-period-seconds}<br> | |
| * Default: 15<br> | |
| * Note: Property override requires a configurable {@link A2AConfigProvider} on the classpath. | |
| */ | |
| long gracePeriodSeconds; | |
| @PostConstruct | |
| void initConfig() { | |
| gracePeriodSeconds = Long.parseLong(configProvider.getValue("a2a.replication.grace-period-seconds")); | |
| } | |
| @Inject | |
| A2AConfigProvider configProvider; | |
| private static final String REPLICATION_GRACE_PERIOD_SECONDS_KEY = "a2a.replication.grace-period-seconds"; | |
| /** | |
| * Grace period for task finalization in replicated scenarios (seconds). | |
| * After a task reaches a final state, this is the minimum time to wait before cleanup | |
| * to allow replicated events to arrive and be processed. | |
| * <p> | |
| * Property: {@code a2a.replication.grace-period-seconds}<br> | |
| * Default: 15<br> | |
| * Note: Property override requires a configurable {@link A2AConfigProvider} on the classpath. | |
| */ | |
| long gracePeriodSeconds; | |
| @PostConstruct | |
| void initConfig() { | |
| gracePeriodSeconds = Long.parseLong(configProvider.getValue(REPLICATION_GRACE_PERIOD_SECONDS_KEY)); | |
| } |
There was a problem hiding this comment.
Will fix upstream. It was already reviewed and merged there
| @PostConstruct | ||
| void initConfig() { | ||
| agentCompletionTimeoutSeconds = Integer.parseInt( | ||
| configProvider.getValue("a2a.blocking.agent.timeout.seconds")); | ||
| consumptionCompletionTimeoutSeconds = Integer.parseInt( | ||
| configProvider.getValue("a2a.blocking.consumption.timeout.seconds")); | ||
| } |
There was a problem hiding this comment.
To improve maintainability and avoid magic strings, it's best to declare configuration keys as constants. Ideally, these would be private static final String fields on the class. This prevents typos and centralizes the key definitions.
@PostConstruct
void initConfig() {
final String AGENT_TIMEOUT_KEY = "a2a.blocking.agent.timeout.seconds";
final String CONSUMPTION_TIMEOUT_KEY = "a2a.blocking.consumption.timeout.seconds";
agentCompletionTimeoutSeconds = Integer.parseInt(
configProvider.getValue(AGENT_TIMEOUT_KEY));
consumptionCompletionTimeoutSeconds = Integer.parseInt(
configProvider.getValue(CONSUMPTION_TIMEOUT_KEY));
}There was a problem hiding this comment.
Will fix upstream. It was already reviewed and merged there
1 participant
Default values will now be used by default. You can supply your own by providing a CDI bean with a higher priority.
Also, there is a new a2a-java-sdk-microprofile-config with the previous MicroProfile Config capabilities. If used, this will allow MicroProfile Config configurations of the properties. The reference implementations use this new module
Fixes #467 🦕
Upstream: #468