Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[CT-1519] [Bug] Better error message for misspecified dist key yaml config #224

Open
2 tasks done
dluftspring opened this issue Nov 16, 2022 · 4 comments · May be fixed by #226
Open
2 tasks done

[CT-1519] [Bug] Better error message for misspecified dist key yaml config #224

dluftspring opened this issue Nov 16, 2022 · 4 comments · May be fixed by #226
Labels
good_first_issue Good for newcomers

Comments

@dluftspring
Copy link

Is this a new bug in dbt-redshift?

  • I believe this is a new bug in dbt-redshift
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

If you set the dist key in a model.yml file like

version: 2
models:
  - name: a_real_table
    config:
      sort:
        - primary_key
        - foreign_key
      dist:
        - primary_key

dbt will throw the following error

list has no attribute strip

Which is generic and non descriptive of what the user should fix. The issue is that since dist keys can only be single valud properties it should be specified as dist: primary_key.

Expected Behavior

A better message that isn't exposing the underlying python machinery and indicates what the user should do to fix the issue e.g. Single valued properties cannot be specified as a list

Steps To Reproduce

See Current Behaviour. If you set up a model like that in any dbt project you will reproduce the error

Relevant log output

No response

Environment

- OS: Mac
- Python: 3.9
- dbt-core:
- dbt-redshift: 1.3.0

Additional Context

No response
@dluftspring dluftspring added bug Something isn't working triage labels Nov 16, 2022
@github-actions github-actions bot changed the title [Bug] Better error message for misspecified dist key yaml config [CT-1519] [Bug] Better error message for misspecified dist key yaml config Nov 16, 2022
@dbeatty10 dbeatty10 self-assigned this Nov 22, 2022
@dbeatty10
Copy link
Contributor

@dluftspring thank you for noticing and reporting this!

You are spot-on with your assessment, and it definitely makes sense to proactively notice where the type of input is unexpected and indicate how to fix it.

I'm going to label this as a "good_first_issue" for a contributor to pick up.

Implementation suggestion

Raise an error at the beginning of this macro if dist is a list instead of a string:

dbt-redshift/dbt/include/redshift/macros/adapters.sql

Lines 2 to 14 in 717018a

{% macro dist(dist) %}
{%- if dist is not none -%}
{%- set dist = dist.strip().lower() -%}
{%- if dist in ['all', 'even'] -%}
diststyle {{ dist }}
{%- elif dist == "auto" -%}
{%- else -%}
diststyle key distkey ({{ dist }})
{%- endif -%}
{%- endif -%}
{%- endmacro -%}

@dbeatty10 dbeatty10 added good_first_issue Good for newcomers and removed triage labels Nov 22, 2022
@dbeatty10 dbeatty10 removed their assignment Nov 22, 2022
@dluftspring
Copy link
Author

If that's all that needs to be done I can contribute the fix for this

@dbeatty10
Copy link
Contributor

Awesome @dluftspring !

When you open up a PR for this, just add the following as the first line:

resolves #224

This will help the reviewer of the PR link back to this conversation.

@dluftspring dluftspring linked a pull request Nov 23, 2022 that will close this issue
Open
6 tasks
@github-actions
Copy link
Contributor

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please remove the stale label or comment on the issue, or it will be closed in 7 days.

@github-actions github-actions bot added the Stale label May 22, 2023
@mikealfare mikealfare added support and removed Stale bug Something isn't working labels Feb 8, 2024
@mikealfare mikealfare reopened this Feb 8, 2024
@Fleid Fleid removed the support label Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good_first_issue Good for newcomers
Projects
None yet
Milestone
No milestone
4 participants
@Fleid @mikealfare @dluftspring @dbeatty10