Redshift connectors#165
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds Trusted IdP types and wiring: regenerated protobuf descriptors and .pyi entries for TRUSTED_IDP and TrustedIDPAuth, request models serialize trusted_idp into protobuf, responses parse trusted_idp back to dicts (expiry → datetime), CoreClient grpc unauthenticated path stops when retries are exhausted, proto ref and SDK version bumped, small fallback and test updates. ChangesTrustedIDP Authorization Support
Supporting and Hygiene Changes
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scalekit/actions/models/responses/get_connected_account_auth_response.py (1)
71-79: ⚡ Quick winAdd Trusted IdP decode parity tests in
tests/test_actions.py.Please add a
ConnectedAccount.from_proto()test for thetrusted_idponeof (fields +expiryconversion), mirroring the existinggoogle_dwdcoverage pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scalekit/actions/models/responses/get_connected_account_auth_response.py` around lines 71 - 79, Add a unit test that exercises ConnectedAccount.from_proto() for the trusted_idp oneof mirroring the google_dwd test pattern: construct a proto_account with authorization_details.trusted_idp populated (db_user, access_key_id, secret_access_key, session_token and expiry), call ConnectedAccount.from_proto(proto_account) and assert the resulting ConnectedAccount.authorization_details["trusted_idp"] fields match the input values and that expiry was converted via ToDatetime() (or is None when unset); follow the existing google_dwd test structure and naming to ensure parity and coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scalekit/actions/models/requests/create_connected_account_request.py`:
- Around line 60-70: In the branch that handles
self.authorization_details["trusted_idp"] (inside
create_connected_account_request), stop coercing missing keys to empty strings;
instead validate that required fields (e.g., access_key_id and
secret_access_key, and db_user if required) exist and are non-empty strings and
raise a ValueError describing the missing/empty key(s). Replace
idp_data.get("...", "") calls with explicit presence checks on idp_data and pass
the actual values to TrustedIDPAuth (do not default to ""), keeping
AuthorizationDetails(trusted_idp=trusted_idp) as before.
In `@scalekit/core.py`:
- Line 168: When promoting a caught grpc.RpcError, preserve it as the causal
exception by raising the promoted exception with explicit exception chaining;
modify the raise that currently calls ScalekitServerException.promote(exp) so
the new exception is raised "from" the original grpc.RpcError (the caught
variable exp) to maintain the original traceback and cause information for
debugging.
---
Nitpick comments:
In `@scalekit/actions/models/responses/get_connected_account_auth_response.py`:
- Around line 71-79: Add a unit test that exercises
ConnectedAccount.from_proto() for the trusted_idp oneof mirroring the google_dwd
test pattern: construct a proto_account with authorization_details.trusted_idp
populated (db_user, access_key_id, secret_access_key, session_token and expiry),
call ConnectedAccount.from_proto(proto_account) and assert the resulting
ConnectedAccount.authorization_details["trusted_idp"] fields match the input
values and that expiry was converted via ToDatetime() (or is None when unset);
follow the existing google_dwd test structure and naming to ensure parity and
coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52bc183c-769a-480c-876e-d1ed16db0e9d
📒 Files selected for processing (9)
scalekit/_version.pyscalekit/actions/models/requests/create_connected_account_request.pyscalekit/actions/models/requests/update_connected_account_request.pyscalekit/actions/models/responses/get_connected_account_auth_response.pyscalekit/core.pyscalekit/v1/connected_accounts/connected_accounts_pb2.pyscalekit/v1/connected_accounts/connected_accounts_pb2.pyiscalekit/v1/connections/connections_pb2.pyscalekit/v1/connections/connections_pb2.pyi
AkshayParihar33
approved these changes
Jun 8, 2026
…mission exception type Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Add Request response for Redshift connectors
Summary by CodeRabbit
New Features
Bug Fixes
Chores