Skip to content

Conversation

@tkislan
Copy link
Contributor

@tkislan tkislan commented Jan 14, 2026

Warning actually gets propagated to Deepnote cloud UI .. which might confuse user
Just hide for now, until we implement this

image

Summary by CodeRabbit

  • Chores
    • Refined internal logging behavior for the Snowflake integration authentication (no user-facing behavior changes).
  • Tests
    • Updated unit tests to reflect the adjusted logging behavior while preserving verification that Snowflake federated auth fetches credentials and leaves connection parameters unchanged.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 14, 2026

📝 Walkthrough

Walkthrough

The logger.warning call for integrationType == "snowflake" was removed from _handle_federated_auth_params and replaced with a no-op (comment + pass). Tests were updated to stop patching the logger and now only mock _get_federated_auth_credentials. There are no changes to public signatures or runtime behavior beyond no longer emitting that warning.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title accurately describes the main change: removing a warning log for unsupported Snowflake federated auth, which is the core modification across both implementation and test files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.



📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 44ecf23 and f242d03.

📒 Files selected for processing (1)
  • tests/unit/test_sql_execution.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

📄 CodeRabbit inference engine (.cursorrules)

**/*.py: Write clean, readable Python code following PEP 8 style guide
Use type hints with Optional[T] for parameters that can be None (not T = None)
Use explicit type hints for function parameters and return values
Maximum line length: 88 characters (Black default)
Use f-strings instead of .format() for string formatting
Use pathlib.Path for file path operations instead of os.path
Use black for code formatting
Use isort for import sorting (black profile)
Use flake8 for linting
Use early returns to reduce nesting and extract common checks into variables for readability
Use snake_case for variable and function names
Use PascalCase for class names
Use snake_case for file names
Support Python versions 3.9, 3.10, 3.11, 3.12, and 3.13

**/*.py: Follow PEP 8 with Black formatting (line length: 88)
Use isort with Black profile for import sorting
Use type hints consistently
Use docstrings for all functions/classes
Use f-strings instead of .format() for string formatting
Use pathlib.Path for file path operations instead of os.path
Always use Optional[T] for parameters that can be None (not T = None)
Use explicit type hints for function parameters and return values
Use snake_case for files, functions, and variables
Use PascalCase for classes
Use appropriate exception types with context logging for error handling
Handle Jupyter/IPython specific exceptions properly
Use early returns to reduce nesting and extract common checks into variables for readability
Use dictionary unpacking for headers (e.g., headers = {"Content-Type": "application/json", **auth_headers})
Use space-separated format for CLI arguments (e.g., --port 8080)

Files:

  • tests/unit/test_sql_execution.py
**/test_*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Name test files with test_*.py pattern

Files:

  • tests/unit/test_sql_execution.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Build and push artifacts for Python 3.10
  • GitHub Check: Build and push artifacts for Python 3.12
  • GitHub Check: Build and push artifacts for Python 3.13
  • GitHub Check: Build and push artifacts for Python 3.11
  • GitHub Check: Build and push artifacts for Python 3.9
  • GitHub Check: Test - Python 3.9
  • GitHub Check: Test - Python 3.11
  • GitHub Check: Test - Python 3.10
🔇 Additional comments (1)
tests/unit/test_sql_execution.py (1)

724-760: LGTM!

Test correctly updated: logger mock removed, credentials fetch verified, params unchanged assertion retained. Covers the no-op behavior for Snowflake federated auth.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Jan 14, 2026

📦 Python package built successfully!

  • Version: 1.2.0.dev6+f84ad11
  • Wheel: deepnote_toolkit-1.2.0.dev6+f84ad11-py3-none-any.whl
  • Install:
    pip install "deepnote-toolkit @ https://deepnote-staging-runtime-artifactory.s3.amazonaws.com/deepnote-toolkit-packages/1.2.0.dev6%2Bf84ad11/deepnote_toolkit-1.2.0.dev6%2Bf84ad11-py3-none-any.whl"

@codecov
Copy link

codecov bot commented Jan 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.27%. Comparing base (2f838ef) to head (f242d03).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #55      +/-   ##
==========================================
- Coverage   73.27%   73.27%   -0.01%     
==========================================
  Files          93       93              
  Lines        5194     5193       -1     
  Branches      758      757       -1     
==========================================
- Hits         3806     3805       -1     
  Misses       1144     1144              
  Partials      244      244              
Flag Coverage Δ
combined 73.27% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🤖 Fix all issues with AI agents
In `@deepnote_toolkit/sql/sql_execution.py`:
- Around line 344-346: The branch that checks federated_auth.integrationType ==
"snowflake" currently does a silent no-op with pass; replace pass with a debug
log statement (e.g., logger.debug) in the sql_execution flow so execution is
traceable without user-facing noise. Log a concise message including the
integration type and that Snowflake federated auth is not supported yet and that
the code will fall back to the original connection URL (include the variable
names federated_auth.integrationType and the connection URL variable used in
this scope). Use the existing module/class logger instance (e.g., logger or
self.logger) to avoid adding new globals.
📜 Review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2f838ef and 44ecf23.

📒 Files selected for processing (1)
  • deepnote_toolkit/sql/sql_execution.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py

📄 CodeRabbit inference engine (.cursorrules)

**/*.py: Write clean, readable Python code following PEP 8 style guide
Use type hints with Optional[T] for parameters that can be None (not T = None)
Use explicit type hints for function parameters and return values
Maximum line length: 88 characters (Black default)
Use f-strings instead of .format() for string formatting
Use pathlib.Path for file path operations instead of os.path
Use black for code formatting
Use isort for import sorting (black profile)
Use flake8 for linting
Use early returns to reduce nesting and extract common checks into variables for readability
Use snake_case for variable and function names
Use PascalCase for class names
Use snake_case for file names
Support Python versions 3.9, 3.10, 3.11, 3.12, and 3.13

**/*.py: Follow PEP 8 with Black formatting (line length: 88)
Use isort with Black profile for import sorting
Use type hints consistently
Use docstrings for all functions/classes
Use f-strings instead of .format() for string formatting
Use pathlib.Path for file path operations instead of os.path
Always use Optional[T] for parameters that can be None (not T = None)
Use explicit type hints for function parameters and return values
Use snake_case for files, functions, and variables
Use PascalCase for classes
Use appropriate exception types with context logging for error handling
Handle Jupyter/IPython specific exceptions properly
Use early returns to reduce nesting and extract common checks into variables for readability
Use dictionary unpacking for headers (e.g., headers = {"Content-Type": "application/json", **auth_headers})
Use space-separated format for CLI arguments (e.g., --port 8080)

Files:

  • deepnote_toolkit/sql/sql_execution.py
deepnote_toolkit/**/*.py

📄 CodeRabbit inference engine (.cursorrules)

deepnote_toolkit/**/*.py: Use dictionary unpacking for headers: headers = {"Content-Type": "application/json", **auth_headers}
Use appropriate exception types, log errors with context, and handle Jupyter/IPython specific exceptions properly
Document functions and classes with docstrings

Files:

  • deepnote_toolkit/sql/sql_execution.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Build and push artifacts for Python 3.11
  • GitHub Check: Typecheck - 3.10
  • GitHub Check: Test - Python 3.9
  • GitHub Check: Build and push artifacts for Python 3.9
  • GitHub Check: Test - Python 3.11
  • GitHub Check: Build and push artifacts for Python 3.12
  • GitHub Check: Test - Python 3.13
  • GitHub Check: Test - Python 3.10
  • GitHub Check: Build and push artifacts for Python 3.10
  • GitHub Check: Build and push artifacts for Python 3.13
  • GitHub Check: Test - Python 3.12

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@deepnote-bot
Copy link

🚀 Review App Deployment Started

📝 Description 🌐 Link / Info
🌍 Review application ra-55
🔑 Sign-in URL Click to sign-in
📊 Application logs View logs
🔄 Actions Click to redeploy
🚀 ArgoCD deployment View deployment
Last deployed 2026-01-14 21:50:58 (UTC)
📜 Deployed commit 9670428490d19200ee6a56e1f9713d0440230fb5
🛠️ Toolkit version f84ad11

@tkislan tkislan marked this pull request as ready for review January 15, 2026 07:55
@tkislan tkislan requested a review from a team as a code owner January 15, 2026 07:55
@tkislan tkislan requested review from m1so and mfranczel January 15, 2026 07:55
@tkislan tkislan merged commit 6de286f into main Jan 15, 2026
35 checks passed
@tkislan tkislan deleted the tk/remove-federated-auth-snowflake-warning branch January 15, 2026 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants