AUT-56 | Add decorator: automation_activity #829
Conversation
This file contains hidden or 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
jnkrmg
changed the title
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 75.7%) Detailed Coverage Report |
📦 Trivy Vulnerability Scan Results
Report Summary
Scan Result Details✅ No vulnerabilities found during the scan for |
📦 Trivy Secret Scan Results
Report Summary
Scan Result Details✅ No secrets found during the scan for |
|
🛠 Docs available at: https://k.atlan.dev/application-sdk/activity_registration |
Collaborator
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
|
🛠 Full Test Coverage Report: https://k.atlan.dev/coverage/application-sdk/pr/829 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| if non_none_types: | ||
| schema = _type_to_json_schema(non_none_types[0]) | ||
| schema["nullable"] = True | ||
| return schema |
There was a problem hiding this comment.
Bug: Non-Optional Union types return empty schema
When processing Union types that don't include None (like Union[str, int]), the code checks if type(None) in args at line 33, which evaluates to False, causing the function to fall through without returning a schema. This results in an empty schema {} being returned instead of a proper union schema. Non-Optional union types are valid Python type hints but are completely ignored by this implementation.
jnkrmg
force-pushed
the
activity_registration
branch
from
48e4356 to
67ba0b1
Compare
jnkrmg
force-pushed
the
activity_registration
branch
from
dd54a2e to
4aade55
Compare
|
|
||
| from application_sdk.decorators.automation_activity import ACTIVITY_SPECS | ||
|
|
||
| __all__: list[str] = ["ACTIVITY_SPECS"] |
There was a problem hiding this comment.
Bug: Decorator not exported for user imports
The __all__ list only exports ACTIVITY_SPECS but not the automation_activity decorator function itself. Users cannot import the decorator to actually use it, defeating the entire purpose of the module. The decorator function must be included in the exports.
jnkrmg
force-pushed
the
activity_registration
branch
from
4aade55 to
7c517b0
Compare
| return | ||
|
|
||
| logger.info( | ||
| f"Registering {len(ACTIVITY_SPECS)} activities with automation engine" |
There was a problem hiding this comment.
Bug: Wrong variable used in log message
The log message incorrectly references the global ACTIVITY_SPECS instead of the activity_specs parameter passed to the function. This causes the logged count to reflect all globally registered activities rather than the specific activities being registered in this call, potentially misleading during debugging.
jnkrmg
force-pushed
the
activity_registration
branch
from
7c517b0 to
aa041c6
Compare
| activity_specs=ACTIVITY_SPECS, | ||
| ) | ||
|
|
||
| asyncio.create_task(_register_activities_with_delay()) |
There was a problem hiding this comment.
Bug: Background task lacks exception handler
The background task created with asyncio.create_task has no exception handler. If flush_activity_registrations raises an exception (which it does at lines 205-207 in automation_activity.py), it becomes an unhandled exception that will only surface when the event loop terminates. This contradicts the "non-blocking" intent stated in the comment and PR description. The task should wrap the call in try-except to log failures without crashing.
jnkrmg
force-pushed
the
activity_registration
branch
from
aa041c6 to
69b0622
Compare
When setup_workflow is called, we will register those activities which are decorated w @automation_activity(). This will register them into Atlas for discoverability in the automation engine
jnkrmg
force-pushed
the
activity_registration
branch
from
69b0622 to
f349543
Compare
| async def _register_activities_with_delay(): | ||
| """Register activities after a 5 second delay to allow automation | ||
| engine server to start.""" | ||
| await asyncio.sleep(1) |
There was a problem hiding this comment.
Bug: Comment states 5 seconds but code sleeps 1
The comment on line 158 states "The 5 second delay allows the automation engine's server to come up" and the docstring on line 160 says "after a 5 second delay", but the actual sleep on line 162 is only 1 second. This mismatch suggests either incomplete testing or a last-minute change that wasn't reflected in documentation.
| activity_specs=ACTIVITY_SPECS, | ||
| ) | ||
|
|
||
| asyncio.create_task(_register_activities_with_delay()) |
There was a problem hiding this comment.
Bug: Fire-and-forget task risks early garbage collection
Creating a task without storing a reference to it risks the task being garbage collected before completion in some Python implementations. While CPython typically keeps running tasks alive, storing the task reference ensures the registration completes reliably across all implementations, similar to how _token_refresh_task is stored in the temporal client.
jnkrmg
force-pushed
the
activity_registration
branch
from
e9ea5a9 to
4e6fed6
Compare
| activity_specs=ACTIVITY_SPECS, | ||
| ) | ||
|
|
||
| asyncio.create_task(_register_activities_with_delay()) |
There was a problem hiding this comment.
Bug: Background task exceptions silently ignored
The background task created with asyncio.create_task has no exception handler or reference saved, so exceptions raised by flush_activity_registrations will be silently lost. While the function catches and re-raises exceptions on line 207-209, these will become unhandled exceptions that only appear in asyncio warnings, making debugging difficult.
|
|
||
|
|
||
| # Global registry to collect decorated activities | ||
| ACTIVITY_SPECS: List[dict[str, Any]] = [] |
There was a problem hiding this comment.
Bug: Activity registry shared across multiple apps
The global ACTIVITY_SPECS list is shared across all application instances in the same Python process. When multiple apps register activities, they'll all be added to the same registry, causing each app to register not only its own activities but also activities from other apps, leading to incorrect registrations.
jnkrmg
force-pushed
the
activity_registration
branch
3 times, most recently
from
c563b2c to
5ccad14
Compare
jnkrmg
force-pushed
the
activity_registration
branch
from
5ccad14 to
cea3f91
Compare
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.
When
app.setup_workflow(...)is called, we will register those activities which are decorated w@automation_activity(...).This will register them into Atlas for discoverability in the automation engine
Changelog
setup_workflowmethod to register activities present in the in-mem registry (ACTIVITY_REGISTRY)ACTIVITY_REGISTRYAdditional context (e.g. screenshots, logs, links)
Checklist
Callouts
Note
Adds a decorator to capture activity schemas and registers them to the automation engine via a non-blocking HTTP call during workflow setup, with new env/config for the engine URL.
application_sdk/decorators/automation_activity.pywith@automation_activity(...)to collect activityname,description,input_schema,output_schema(via reflection/Pydantic), stored inACTIVITY_SPECS.flush_activity_registrations(...)to POST collected tools toPOST /api/toolsafter a health check on/api/health.application_sdk/application/__init__.py, schedules a delayed, non-blocking registration task duringsetup_workflow(...)usingflush_activity_registrationswithACTIVITY_SPECS.AUTOMATION_ENGINE_API_HOST,AUTOMATION_ENGINE_API_PORT, and derivedAUTOMATION_ENGINE_API_URLinapplication_sdk/constants.py..env.examplewith the new automation engine host/port variables.Written by Cursor Bugbot for commit cea3f91. This will update automatically on new commits. Configure here.