[WIP] Rename agent to connector #3165
Conversation
Signed-off-by: Kevin Su <[email protected]>
pingsutw
requested review from
fg91,
wild-endeavor,
kumare3,
eapolinario,
cosmicBboy,
samhita-alla,
thomasjpfan and
Future-Outlier
as code owners
Code Review Agent Run #a98aa0Actionable Suggestions - 2
Review Details
AI Code Review powered by |
Changelist by BitoThis pull request implements the following key changes.
|
| @@ -94,4 +94,4 @@ def delete(self, resource_meta: BigQueryMetadata, **kwargs): | |||
| client.cancel_job(resource_meta.job_id, resource_meta.project, resource_meta.location) | |||
|
|
|||
|
|
|||
| AgentRegistry.register(BigQueryAgent()) | |||
| AgentRegistry.register(BigQueryConnector()) | |||
There was a problem hiding this comment.
The class name BigQueryConnector is being used in the registration call, but based on the surrounding code context, it appears the class is actually named BigQueryAgent. This mismatch will likely cause runtime errors.
Code suggestion
Check the AI-generated fix before applying
| AgentRegistry.register(BigQueryConnector()) | |
| AgentRegistry.register(BigQueryAgent()) |
Code Review Run #a98aa0
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Code Review Agent Run #71e148Actionable Suggestions - 2
Review Details
AI Code Review powered by |
| @@ -110,12 +110,12 @@ def say_hello0(name: str) -> str: | |||
| return f"Hello, {name}." | |||
|
|
|||
| task_spec = get_serializable(OrderedDict(), serialization_settings, say_hello0) | |||
| agent = AgentRegistry.get_agent(task_spec.template.type) | |||
| connector = ConnectorRegistry.get_connector(task_spec.template.type) | |||
There was a problem hiding this comment.
It appears that AgentRegistry has been renamed to ConnectorRegistry in this PR. This change should be reflected in the imports as well. Currently, the import statement for ConnectorRegistry is missing, which could lead to runtime errors.
Code suggestion
Check the AI-generated fix before applying
@@ -9,6 +9,7 @@
from flyteidl.core.execution_pb2 import TaskExecution
from flytekitplugins.mmcloud import MMCloudConnector, MMCloudConfig, MMCloudTask
from flytekitplugins.mmcloud.utils import async_check_output, flyte_to_mmcloud_resources
+from flytekit.extend.backend.base_connector import ConnectorRegistry
from flytekit import Resources, task
from flytekit.configuration import DefaultImages, ImageConfig, SerializationSettings
from flytekit.extend import get_serializable
Code Review Run #71e148
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
|
|
||
| assert isinstance(agent, MMCloudAgent) | ||
| assert isinstance(connector, MMCloudConnector) |
There was a problem hiding this comment.
The assertion is checking for MMCloudConnector but there's no import for this class. The import statement should be updated to include MMCloudConnector instead of or in addition to MMCloudAgent.
Code suggestion
Check the AI-generated fix before applying
@@ -10,6 +10,7 @@
from flyteidl.core.execution_pb2 import TaskExecution
from flytekitplugins.mmcloud import MMCloudConnector, MMCloudConfig, MMCloudTask
from flytekitplugins.mmcloud.utils import async_check_output, flyte_to_mmcloud_resources
+from flytekit.extend.backend.base_connector import ConnectorRegistry
from flytekit import Resources, task
from flytekit.configuration import DefaultImages, ImageConfig, SerializationSettings
from flytekit.extend import get_serializable
Code Review Run #71e148
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Signed-off-by: Kevin Su <[email protected]>
Code Review Agent Run #c2bc44Actionable Suggestions - 0Review Details
AI Code Review powered by |
Signed-off-by: Kevin Su <[email protected]>
Code Review Agent Run #64ed85Actionable Suggestions - 0Review Details
AI Code Review powered by |
Signed-off-by: Kevin Su <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3165 +/- ##
==========================================
- Coverage 78.74% 75.30% -3.45%
==========================================
Files 321 214 -107
Lines 27055 22261 -4794
Branches 2901 2901
==========================================
- Hits 21305 16764 -4541
+ Misses 4947 4647 -300
- Partials 803 850 +47 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Code Review Agent Run #d82c2bActionable Suggestions - 0Review Details
AI Code Review powered by |
Signed-off-by: Kevin Su <[email protected]>
Code Review Agent Run #5dcdceActionable Suggestions - 0Review Details
AI Code Review powered by |
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Code Review Agent Run #003d61Actionable Suggestions - 0Review Details
AI Code Review powered by |
Signed-off-by: Kevin Su <[email protected]>
Code Review Agent Run #c18704Actionable Suggestions - 0Review Details
|
Tracking issue
Why are the changes needed?
What changes were proposed in this pull request?
How was this patch tested?
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link
Summary by Bito
This pull request systematically renames 'Agent' to 'Connector' across the entire codebase for consistent terminology and improved maintainability. Changes span core backend modules, CLI commands, webhook components, and multiple plugins including Airflow, AWS SageMaker, BigQuery, and k8sdataservice. This terminology standardization reduces ambiguity, clarifies module responsibilities, and enhances overall code structure and maintainability.Unit tests added: False
Estimated effort to review (1-5, lower is better): 5