-
Notifications
You must be signed in to change notification settings - Fork 75
Connection Description #1147
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
base: main
Are you sure you want to change the base?
Connection Description #1147
Conversation
WalkthroughAdds an optional description field to ConnectionMeta and Airbyte connection schemas, plus a migration. Airbyte helper flows (create/get/get_one/update) persist and surface descriptions, including bulk retrieval via a description_map. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Airbyte Helpers
participant DB as ConnectionMeta (DB)
participant AB as Airbyte API
Client->>API: create_connection(payload{name, description,...})
API->>AB: Create Airbyte connection
AB-->>API: Connection details (id, ...)
API->>DB: Save {connection_id, connection_name, description}
API-->>Client: Response including description
sequenceDiagram
participant Client
participant API as Airbyte Helpers
participant DB as ConnectionMeta (DB)
participant AB as Airbyte API
Client->>API: get_connections()
API->>AB: List connections
AB-->>API: connections[]
API->>DB: Fetch descriptions for connection_ids
DB-->>API: description_map
API-->>Client: connections[] with description fields
sequenceDiagram
participant Client
participant API as Airbyte Helpers
participant DB as ConnectionMeta (DB)
participant AB as Airbyte API
Client->>API: update_connection({name?, description?})
API->>AB: Update Airbyte connection (if name present)
API->>DB: Update ConnectionMeta {connection_name?, description?}
API-->>Client: Updated connection including description
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1147 +/- ##
==========================================
+ Coverage 70.42% 70.44% +0.01%
==========================================
Files 86 86
Lines 8275 8289 +14
==========================================
+ Hits 5828 5839 +11
- Misses 2447 2450 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (5)
ddpui/models/org.py (1)
213-218
: Add API-level length validation for description to prevent DB errorsThe model caps description at 100 chars. Currently, services write payload.description directly, which can raise IntegrityError if clients send longer strings. Recommend enforcing max length at the schema layer and/or sanitizing at write points.
- Option A (preferred): enforce max length in API schemas with pydantic constraints.
- Option B: defensively truncate before saving (see suggested diffs in airbytehelpers comments).
ddpui/ddpairbyte/airbytehelpers.py (3)
281-309
: Sanitize and consistently return the stored description on createTwo small issues:
- No guard against >100 chars before persisting to ConnectionMeta (DB error risk).
- The response echoes payload.description; if you truncate/sanitize before saving, the response should reflect what’s actually stored.
Proposed fix: sanitize once and reuse for both save and response.
ConnectionMeta.objects.create( - connection_id=airbyte_conn["connectionId"], - connection_name=payload.name, - description=payload.description, + connection_id=airbyte_conn["connectionId"], + connection_name=payload.name, + description=(payload.description[:100] if payload.description else None), ) @@ "clearConnDeploymentId": clear_dataflow.deployment_id, - "description": payload.description, + "description": (payload.description[:100] if payload.description else None), }Alternatively, compute a local variable once above the create and reuse it in both places.
393-397
: Micro-optimization: fetch only needed fields for description_mapAvoid materializing full model instances. Using values_list improves memory and slightly reduces query overhead.
-connection_metas = ConnectionMeta.objects.filter(connection_id__in=connection_ids) -description_map = {meta.connection_id: meta.description for meta in connection_metas} +connection_metas = ConnectionMeta.objects.filter(connection_id__in=connection_ids) \ + .values_list("connection_id", "description") +description_map = dict(connection_metas)
593-596
: LGTM with a tiny optional tweakFetching description via a single query is fine. If you want to be extra lean, fetch only the field you need:
-connection_meta = ConnectionMeta.objects.filter(connection_id=connection_id).first() +connection_meta = ConnectionMeta.objects.filter(connection_id=connection_id).only("description").first()ddpui/ddpairbyte/schema.py (1)
69-69
: Enforce max length (100) at the schema level to prevent DB write failuresAdd a string constraint so overly long descriptions are rejected upfront by validation instead of failing at DB time.
Apply this diff to the changed fields:
- description: str = None + description: Optional[constr(max_length=100)] = NoneDo this for:
- AirbyteConnectionCreate.description (Line 69)
- AirbyteConnectionCreateResponse.description (Line 88)
- AirbyteGetConnectionsResponse.description (Line 107)
- AirbyteConnectionUpdate.description (Line 119)
And add the import:
# add alongside other imports at top of file from pydantic import constrAlso applies to: 88-88, 107-107, 119-119
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
ddpui/ddpairbyte/airbytehelpers.py
(7 hunks)ddpui/ddpairbyte/schema.py
(4 hunks)ddpui/migrations/0128_add_description_to_connection_meta.py
(1 hunks)ddpui/models/org.py
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
ddpui/ddpairbyte/airbytehelpers.py (1)
ddpui/models/org.py (1)
ConnectionMeta
(206-218)
🔇 Additional comments (2)
ddpui/migrations/0128_add_description_to_connection_meta.py (1)
12-21
: Migration aligns with the model; LGTMField definition matches the model (CharField, max_length=100, blank=True, null=True, help_text). Dependency on 0127 looks correct. No data backfill needed since the field is nullable.
ddpui/ddpairbyte/airbytehelpers.py (1)
513-514
: LGTM: description is included in bulk get responsePropagating description via the preloaded map is correct and avoids per-item DB lookups.
…nection functions
Summary by CodeRabbit
New Features
Chores