handle ssl cert in dbt cli config#1323
Merged
Ishankoradia merged 4 commits intomainfrom May 4, 2026
Merged
Conversation
WalkthroughDBT SSL mapping now sets ChangesSSL Certificate Content Injection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
siddhant3030
reviewed
May 4, 2026
Contributor
siddhant3030
left a comment
siddhant3030
left a comment
There was a problem hiding this comment.
ddpui/core/dbtfunctions.py:48 — sslrootcert_content isn't consumed anywhere in DDP_backend. Is there a paired prefect-proxy PR that reads this key and writes the cert to disk before dbt runs? Without it, sslrootcert will point to a file that doesn't exist.
![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)
ddpui/tests/core/test_dbtfunctions.py (1)
126-130: ⚡ Quick win
assert Falseis stripped by-Oand the bareExceptioncatch hides unrelated errors (Ruff B011 / BLE001)Use
pytest.raisesfor both correctness and clarity:♻️ Proposed refactor
- try: - map_airbyte_destination_spec_to_dbtcli_profile(conn_info, dbt_project_params) - assert False, "should have raised" - except Exception as e: - assert "org_project_dir is required" in str(e) + import pytest + with pytest.raises(Exception, match="org_project_dir is required"): + map_airbyte_destination_spec_to_dbtcli_profile(conn_info, dbt_project_params)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ddpui/tests/core/test_dbtfunctions.py` around lines 126 - 130, The test currently uses a try/except with assert False and a bare Exception which is fragile and suppressed by -O; replace it with pytest.raises to explicitly assert the expected error type and message from map_airbyte_destination_spec_to_dbtcli_profile: wrap the call in pytest.raises(ExpectedException, match="org_project_dir is required") (choose the actual exception type raised by the function, e.g., ValueError) so the test fails for unexpected exceptions and correctly verifies the error text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ddpui/core/dbtfunctions.py`:
- Around line 44-48: The code accesses dbt_project_params.org_project_dir
without ensuring dbt_project_params is not None; update the conditional that
checks ca_certificate to first verify dbt_project_params is truthy and then
org_project_dir (e.g., change the check to something like "if not
dbt_project_params or not dbt_project_params.org_project_dir") so that when
dbt_project_params is None you raise the same "org_project_dir is required..."
Exception instead of triggering an AttributeError; reference the ca_certificate
variable and dbt_project_params.org_project_dir in your fix.
- Around line 50-53: conn_info sets sslrootcert_content but no code writes it to
disk, so dbt's sslrootcert path will point to a missing file when sslmode:
verify-ca; fix by writing conn_info["sslrootcert_content"] to the file path in
conn_info["sslrootcert"] (under dbt_project_params.org_project_dir, named
sslrootcert.pem) before the profile is saved or before the dbt CLI subprocess is
invoked. Ensure this write happens prior to calling
update_dbt_cli_profile_block(credentials=dbt_creds) and prior to any code that
renders/dumps profiles.yml so the actual certificate file exists on disk when
dbt runs. Also ensure file permissions are appropriate and handle/raise errors
if the certificate content is missing.
---
Nitpick comments:
In `@ddpui/tests/core/test_dbtfunctions.py`:
- Around line 126-130: The test currently uses a try/except with assert False
and a bare Exception which is fragile and suppressed by -O; replace it with
pytest.raises to explicitly assert the expected error type and message from
map_airbyte_destination_spec_to_dbtcli_profile: wrap the call in
pytest.raises(ExpectedException, match="org_project_dir is required") (choose
the actual exception type raised by the function, e.g., ValueError) so the test
fails for unexpected exceptions and correctly verifies the error text.
🪄 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: 60bd1523-d151-4171-ab8a-a768fbc298a3
📒 Files selected for processing (2)
ddpui/core/dbtfunctions.pyddpui/tests/core/test_dbtfunctions.py
siddhant3030
approved these changes
May 4, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1323 +/- ##
=======================================
Coverage 58.42% 58.42%
=======================================
Files 132 132
Lines 15653 15653
=======================================
Hits 9146 9146
Misses 6507 6507 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ddpui/core/dbtfunctions.py (1)
50-53:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift
sslrootcert.pemstill never written to disk —verify-cadbt runs will fail.
sslrootcert_contentis stored inconn_infoandsslrootcertpoints to a path that doesn't exist. The test intentionally asserts the file is absent at setup time ("runtime cert writing"), but no code in this PR (or anywhere else in the codebase per the previous exhaustive verification) readssslrootcert_contentand materializes the file before the dbt CLI subprocess runs. Everyverify-caconnection will fail with a missing-file error until that write step exists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ddpui/core/dbtfunctions.py` around lines 50 - 53, conn_info currently stores sslrootcert_content and points sslrootcert to a path that doesn't exist; before launching the dbt CLI you must materialize that PEM file. In the function that builds/returns conn_info or immediately before calling the subprocess that runs dbt (locate where conn_info is used to invoke dbt), write conn_info["sslrootcert_content"] to os.path.join(dbt_project_params.org_project_dir, "sslrootcert.pem"), set safe file permissions (e.g., 0600), perform an atomic write (tmp file + rename) and ensure conn_info["sslrootcert"] points to that created file so verify-ca connections can find it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ddpui/tests/core/test_dbtfunctions.py`:
- Around line 111-118: Replace the try/except + assert False pattern in
test_map_airbyte_destination_spec_to_dbtcli_profile_ssl_no_org_project_dir with
pytest.raises to avoid B011 and BLE001: wrap the call to
map_airbyte_destination_spec_to_dbtcli_profile(conn_info, None) in a with
pytest.raises(<expected-exception-type>, match="org_project_dir is required")
block (use the concrete exception that the function raises, e.g., ValueError) so
the test asserts the specific exception and message instead of catching all
exceptions or relying on assert False.
---
Duplicate comments:
In `@ddpui/core/dbtfunctions.py`:
- Around line 50-53: conn_info currently stores sslrootcert_content and points
sslrootcert to a path that doesn't exist; before launching the dbt CLI you must
materialize that PEM file. In the function that builds/returns conn_info or
immediately before calling the subprocess that runs dbt (locate where conn_info
is used to invoke dbt), write conn_info["sslrootcert_content"] to
os.path.join(dbt_project_params.org_project_dir, "sslrootcert.pem"), set safe
file permissions (e.g., 0600), perform an atomic write (tmp file + rename) and
ensure conn_info["sslrootcert"] points to that created file so verify-ca
connections can find it.
🪄 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: 8b57403f-9ff9-4f72-ac1a-9591d0a7e26f
📒 Files selected for processing (3)
ddpui/core/dbtfunctions.pyddpui/ddpdbt/schema.pyddpui/tests/core/test_dbtfunctions.py
✅ Files skipped from review due to trivial changes (1)
- ddpui/ddpdbt/schema.py
Comment on lines
+111
to
+118
| def test_map_airbyte_destination_spec_to_dbtcli_profile_ssl_no_org_project_dir(): | ||
| """Tests ssl with ca_certificate but no dbt_project_params raises""" | ||
| conn_info = {"ssl_mode": {"mode": "verify-ca", "ca_certificate": "ca_certificate"}} | ||
| try: | ||
| map_airbyte_destination_spec_to_dbtcli_profile(conn_info, None) | ||
| assert False, "should have raised" | ||
| except Exception as e: | ||
| assert "org_project_dir is required" in str(e) |
There was a problem hiding this comment.
Replace try/except + assert False with pytest.raises.
Two Ruff-flagged problems here:
- B011 —
assert Falseis silently removed underpython -O, so the "should have raised" sentinel never fires in optimised mode. - BLE001 — bare
except Exceptionswallows any unexpected error (AttributeError,KeyError, etc.) and checks its message against"org_project_dir is required", meaning an unrelated crash causes a false-pass.
pytest.raises fixes both in one move:
🛠️ Proposed fix
+import pytest
+
def test_map_airbyte_destination_spec_to_dbtcli_profile_ssl_no_org_project_dir():
"""Tests ssl with ca_certificate but no dbt_project_params raises"""
conn_info = {"ssl_mode": {"mode": "verify-ca", "ca_certificate": "ca_certificate"}}
- try:
- map_airbyte_destination_spec_to_dbtcli_profile(conn_info, None)
- assert False, "should have raised"
- except Exception as e:
- assert "org_project_dir is required" in str(e)
+ with pytest.raises(Exception, match="org_project_dir is required"):
+ map_airbyte_destination_spec_to_dbtcli_profile(conn_info, None)🧰 Tools
🪛 Ruff (0.15.12)
[warning] 116-116: Do not assert False (python -O removes these calls), raise AssertionError()
Replace assert False
(B011)
[warning] 117-117: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ddpui/tests/core/test_dbtfunctions.py` around lines 111 - 118, Replace the
try/except + assert False pattern in
test_map_airbyte_destination_spec_to_dbtcli_profile_ssl_no_org_project_dir with
pytest.raises to avoid B011 and BLE001: wrap the call to
map_airbyte_destination_spec_to_dbtcli_profile(conn_info, None) in a with
pytest.raises(<expected-exception-type>, match="org_project_dir is required")
block (use the concrete exception that the function raises, e.g., ValueError) so
the test asserts the specific exception and message instead of catching all
exceptions or relying on assert False.
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.
Summary by CodeRabbit
Bug Fixes
Tests