Skip to content

Comments

feat: Add support for dbt_integration_id also#53

Merged
mdesmet merged 4 commits intomainfrom
feat/generic-integration-id
Jun 12, 2025
Merged

feat: Add support for dbt_integration_id also#53
mdesmet merged 4 commits intomainfrom
feat/generic-integration-id

Conversation

@suryaiyer95
Copy link
Collaborator

@suryaiyer95 suryaiyer95 commented May 28, 2025

Important

Add support for dbt_integration_id in onboard command in cli.py.

  • Behavior:
    • Add support for --dbt_integration_id and --dbt_integration_environment options in onboard command in cli.py.
    • Update onboard function to use dbt_integration_id and dbt_integration_environment instead of dbt_core_integration_id and dbt_core_integration_environment.
  • Misc:
    • Update help text for onboard command options to reflect new functionality.

This description was created by Ellipsis for d45f4f3. You can customize this summary. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed everything up to b08db32 in 33 seconds. Click for details.
  • Reviewed 81 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/datapilot/core/platforms/dbt/cli/cli.py:137
  • Draft comment:
    The new click.option aliases for integration ID are correctly added. Consider clarifying the help text to avoid ambiguity between 'DBT Core Integration ID' and 'DBT Integration ID'.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. src/datapilot/core/platforms/dbt/cli/cli.py:155
  • Draft comment:
    Ensure that renaming to 'dbt_integration_id' and 'dbt_integration_environment' is reflected consistently in downstream function calls (onboard_file and start_dbt_ingestion).
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
3. src/datapilot/core/platforms/dbt/cli/cli.py:195
  • Draft comment:
    The URL construction using the new integration parameters appears correct; double-check that the backend expects these new identifiers.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_e7ouObtflda1FA0i

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed d45f4f3 in 46 seconds. Click for details.
  • Reviewed 23 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/datapilot/core/platforms/dbt/cli/cli.py:133
  • Draft comment:
    Unified integration ID option: ensure the help text clearly indicates support for both '--dbt_core_integration_id' and '--dbt_integration_id'.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. src/datapilot/core/platforms/dbt/cli/cli.py:151
  • Draft comment:
    Onboard signature update: confirm that the auth_options decorator now correctly supplies token, instance_name, and backend_url, replacing the unused 'ctx' parameter.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_v7UhYX8OThB5jx1D

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@mdesmet mdesmet self-requested a review June 12, 2025 07:38
@mdesmet mdesmet merged commit 8209c28 into main Jun 12, 2025
39 checks passed
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.

2 participants