feat(agents-api): Make execution id temporal search attribute configurable #1182
+13
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe PR introduces a new environment variable Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts belowEmoji Descriptions:
Interact with the Bot:
|
|
LGTM 👍 |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 7198615 in 1 minute and 23 seconds
More details
- Looked at
85lines of code in5files - Skipped
0files when reviewing. - Skipped posting
11drafted comments based on config settings.
1. .env.example:88
- Draft comment:
ENV var TEMPORAL_SEARCH_ATTRIBUTE_KEY added. Consider adding a brief comment on its purpose. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
2. agents-api/agents_api/clients/temporal.py:116
- Draft comment:
Replacing hardcoded 'CustomStringField' with config variable is good; ensure consistency with docs. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
3. agents-api/agents_api/env.py:116
- Draft comment:
The env var temporal_search_attribute_key is defined with a default. Looks good, ensure secret usage if needed. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
4. agents-api/agents_api/workflows/task_execution/helpers.py:91
- Draft comment:
Using temporal_search_attribute_key from env for SearchAttributeKey is consistent. No action needed. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
5. agents-api/docker-compose.yml:32
- Draft comment:
TEMPORAL_SEARCH_ATTRIBUTE_KEY is now configurable in docker-compose. Verify this is documented in project docs. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
6. .env.example:88
- Draft comment:
Add a brief comment explaining TEMPORAL_SEARCH_ATTRIBUTE_KEY usage. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
7. agents-api/agents_api/clients/temporal.py:116
- Draft comment:
Nice improvement using temporal_search_attribute_key instead of hardcoded value; consider documenting accepted values. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
8. agents-api/agents_api/env.py:116
- Draft comment:
Consider adding an inline comment to explain the purpose and acceptable values of temporal_search_attribute_key. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
9. agents-api/agents_api/workflows/task_execution/helpers.py:91
- Draft comment:
Consistent use of temporal_search_attribute_key enhances flexibility; ensure tests cover behavior with alternative values. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
10. agents-api/docker-compose.yml:32
- Draft comment:
Verify that the default TEMPORAL_SEARCH_ATTRIBUTE_KEY in docker-compose matches code expectations. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
11. agents-api/agents_api/workflows/task_execution/helpers.py:126
- Draft comment:
Typo detected: The variable name 'seprated_workflow_name' should be corrected to 'separated_workflow_name' for consistency and readability. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_kfdWIdQ4z7jdL4wp
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
creatorrr
reviewed
Feb 21, 2025
creatorrr
requested changes
Feb 21, 2025
creatorrr
requested changes
Feb 21, 2025
CI Feedback 🧐(Feedback updated until commit 7198615)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on dfb5964 in 1 minute and 15 seconds
More details
- Looked at
26lines of code in2files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. agents-api/agents_api/env.py:116
- Draft comment:
Ensure the new default value 'CustomStringField' is intentional. It differs from the previous 'JulepExecutionId'; confirm downstream systems rely on this change. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
This comment violates several rules. It's asking to "ensure" and "confirm" something, which is explicitly against the rules. It's speculative about downstream impacts. It's not pointing out a clear issue, just asking for verification. The change is a simple configuration default value which can be overridden by environment variables anyway.
Maybe this is a critical configuration that could break the system if changed incorrectly? Maybe there's domain knowledge about Temporal that makes this change dangerous?
Even if this is an important configuration, asking for verification isn't helpful. If there was a specific known issue, that should be stated directly. The default can be overridden via environment variables if needed.
Delete this comment. It violates the rules by asking for verification without pointing out a specific issue, and makes speculative statements about downstream impacts.
2. agents-api/docker-compose.yml:32
- Draft comment:
Verify that the default value update to 'CustomStringField' in docker-compose aligns with the application's expected configuration. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify alignment with expected configuration, which is against the rules. It doesn't provide a specific suggestion or point out a clear issue.
3. agents-api/agents_api/env.py:116
- Draft comment:
Ensure to update documentation/comments to reflect the new default value for TEMPORAL_SEARCH_ATTRIBUTE_KEY ('CustomStringField') replacing 'JulepExecutionId'. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
4. agents-api/docker-compose.yml:32
- Draft comment:
Verify the updated default value for TEMPORAL_SEARCH_ATTRIBUTE_KEY ('CustomStringField') is consistent with application expectations. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
Workflow ID: wflow_Ujqbk2kTp6Z9HGJI
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 285daec in 24 seconds
More details
- Looked at
20lines of code in2files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. .env.example:88
- Draft comment:
Consider adding a brief comment above TEMPORAL_SEARCH_ATTRIBUTE_KEY to explain its purpose and usage. The default value 'CustomStringField' may not be self-explanatory for new users. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
2. .env.example:88
- Draft comment:
Changed default TEMPORAL_SEARCH_ATTRIBUTE_KEY value to 'CustomStringField'. Ensure this new default is intentional, well-documented, and that downstream integrations are updated accordingly. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
3. code-interpreter/vendor/cohere-ai/cohere-terrarium:1
- Draft comment:
Vendor subproject pointer added. Verify that this commit reference is correct and that any related dependency updates are documented. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
Workflow ID: wflow_pmNda4i42CG2up34
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
we have removed this submodule. Please re-check it once.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on f598c7f in 41 seconds
More details
- Looked at
5lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. code-interpreter/vendor/cohere-ai/cohere-terrarium:1
- Draft comment:
Confirm that the removal of the subproject commit reference is intentional. Removing it might affect vendor dependency reproducibility or be an unintended side-effect. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. code-interpreter/vendor/cohere-ai/cohere-terrarium:1
- Draft comment:
Removal of the submodule commit reference detected. Please confirm that this vendor dependency update is intentional and that the new dependency state is documented. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_45BLUfROAcTtFxcA
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Vedantsahai18
approved these changes
Feb 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Type
Enhancement
Description
Made the Temporal search attribute key configurable.
Updated environment variables and configuration for Temporal.
Refactored code to use the configurable search attribute key.
Updated
.env.exampleanddocker-compose.ymlfor new configuration.Changes walkthrough 📝
temporal.py
Use configurable Temporal search attribute keyagents-api/agents_api/clients/temporal.py
temporal_search_attribute_keyfrom environment variables.helpers.py
Refactor to use configurable search attribute keyagents-api/agents_api/workflows/task_execution/helpers.py
temporal_search_attribute_keyfrom environment variables.env.py
Add Temporal search attribute key to environmentagents-api/agents_api/env.py
temporal_search_attribute_keyto environment variables.docker-compose.yml
Update `docker-compose.yml` with new configurationagents-api/docker-compose.yml
TEMPORAL_SEARCH_ATTRIBUTE_KEYto shared environment variables..env.example
Update `.env.example` with new configuration.env.example
TEMPORAL_SEARCH_ATTRIBUTE_KEY.Important
Make Temporal search attribute key configurable by updating environment variables, configuration files, and refactoring code in
temporal.pyandhelpers.py.temporal_search_attribute_keyintemporal.pyandhelpers.py.temporal_search_attribute_keyfrom environment variables.temporal_search_attribute_keytoenv.pywith a default value.docker-compose.ymlto includeTEMPORAL_SEARCH_ATTRIBUTE_KEYin shared environment variables..env.exampleto includeTEMPORAL_SEARCH_ATTRIBUTE_KEY.This description was created by
for f598c7f. It will automatically update as commits are pushed.