Add regex to star #466

robertdefilippi · 2021-12-18T17:07:38Z

This is a:

bug fix PR with no breaking changes — please ensure the base branch is master
new functionality — please ensure the base branch is the latest dev/ branch
a breaking change — please ensure the base branch is the latest dev/ branch

Description & motivation

Adding regex functionality to dbt_utils.star

Checklist

robertdefilippi · 2021-12-18T17:09:28Z

I'm having some issues testing this from the instructions in the ./integration_tests.

When I try to test I get the following error:

# ./dbt-utils/integration_tests

$ make test target=postgres
docker-compose -f ../docker-compose.yml up dbt
[+] Running 2/2
 ⠿ Container dbt-utils_postgres_1  Created                                                                                                                                   0.0s
 ⠿ Container dbt-utils_dbt_1       Recreated                                                                                                                                 0.2s
Attaching to dbt_1
dbt_1  | Running with dbt=1.0.0-b2
dbt_1  | No profile "integration_tests" found, continuing with no target
dbt_1  | Installing ../
dbt_1  |   Installed from <local @ ../>
dbt_1  | Running with dbt=1.0.0-b2
dbt_1  | Encountered an error while reading profiles:
dbt_1  |   ERROR Runtime Error
dbt_1  |   Compilation Error
dbt_1  |     Could not render {{ env_var('POSTGRES_TEST_HOST') }}: Env var required but not provided: 'POSTGRES_TEST_HOST'
dbt_1  | Defined profiles:
dbt_1  |  - integration_tests
dbt_1  |
dbt_1  | For more information on configuring profiles, please consult the dbt docs:
...

Should the profile integration_tests be loaded when I run the docker container?

How can I fix this so I can test my PR.

Thanks!

joellabes · 2022-01-23T22:06:13Z

@robertdefilippi have a look at the https://github.com/dbt-labs/dbt-utils/blob/main/run_test.sh script which is triggered by Circle.

It looks like the profile is populated by copying from ci/sample.profiles.yml, which doesn't actually exist in this project anymore 😕

I just tried to look at the webhook history for this, but GitHub only retains them for a month.

Could you push something else up to this PR branch? I think that should trigger another webhook and then I can see what happens after that.

I'm broadly on board with bringing this in though, once we get the tests working 👍

robertdefilippi · 2022-01-24T12:20:00Z

@joellabes Just pushed a changed. Would love to figure out what the issues is 😄

joellabes · 2022-01-24T20:43:09Z

OK I've opened an issue with circle ci's support team - the webhook was rejected but I don't know why :(

robertdefilippi · 2022-01-24T21:00:56Z

Not a problem @joellabes ! This has been sitting for a month, so a little longer won't be an issue. Thank you for looking into this 😃

joellabes · 2022-01-27T05:04:40Z

OK I heard back from them! Sounds like if you try again it'll work? 🤞

When checking our logs for the X-GitHub-Delivery id XXXXXX, I saw that we were getting 401 errors for the user robertdefilippi from Github.

This can happen when a user has a CircleCI account, but hasn't logged in to CircleCI in a long time, and we have a stale token cached for them on our end. When this happens, builds may be blocked, and you will see a 400 webhook error on Github.

For PRs created/updated by users who do not have a CircleCI account, this issue should not block your builds.

I have refreshed the cache for the user `robertdefilippi, but could you have them try to push another commit to the branch that has an open PR?

robertdefilippi · 2022-01-27T14:21:43Z

Looks like that worked! Looks like I'll have to keep an eye out on my circle-ci account from now on. Thanks for your help @joellabes

Now I just need to fix these errors 👍

robertdefilippi · 2022-02-06T14:45:34Z

@joellabes finally got this all figured out 🙂 Thanks for all your help through this!

joellabes · 2022-02-09T00:32:26Z

Hurrah! I'll check it out start of next week 🔜

joellabes · 2022-02-21T03:45:09Z

@robertdefilippi sorry for the slow moving here! My current plan is to ship 0.8.1 tomorrow-ish, and then include your changes in 0.9.0 of utils sometime after that (because the change of method signature will be a breaking change for anyone using positional arguments), which will be a wee way away because it will also be where we pull some stuff out of utils as discussed in #487

Fair warning, 0.8.1 includes a fair bit of movement around the star macro - shouldn't be insurmountable but I think you'll find some merge conflicts sorry. It'd be worth changing the target branch to next/minor once 0.8.1 is out.

robertdefilippi · 2022-02-21T22:10:10Z

@joellabes got it! Thanks for all your help so far, and the heads up with the potential changes coming my way 😄

Once 8.1 is out I'll have a go and fixing the conflicts.

* Update README.md * add some flexibility to expression_is_true execution plan and add a few new tests * catch duplicate field name exception when the expression_is_true test is invoked * expression is true - rename seeds, format sql and get rid of dupe column handler from PR comments Co-authored-by: Joel Labes <joel.labes@dbtlabs.com>

dbeatty10 · 2022-04-26T01:33:54Z

macros/sql/star.sql

+{% macro star(from, relation_alias=False, except=[], regex='', prefix='', suffix='') -%}
+    {{ return(adapter.dispatch('star', 'dbt_utils')(from, relation_alias, except, regex, prefix, suffix)) }}
 {% endmacro %}

-{% macro default__star(from, relation_alias=False, except=[], prefix='', suffix='') -%}
+{% macro default__star(from, relation_alias=False, except=[], regex='', prefix='', suffix='') -%}


I'd prefer to add the optional regex argument to the end of the signature. That way, this new feature will be guaranteed to be non-breaking for all users.

Do you have any thoughts about the order of parameters?

I think the change would be as simple as the following:

{% macro star(from, relation_alias=False, except=[], prefix='', suffix='', regex='') -%} {{ return(adapter.dispatch('star', 'dbt_utils')(from, relation_alias, except, prefix, suffix, regex)) }} {% endmacro %} {% macro default__star(from, relation_alias=False, except=[], prefix='', suffix='', regex='') -%}

dbeatty10

I'd prefer to add the optional regex argument to the end of the signature. That way, this new feature will be guaranteed to be non-breaking for all users.

Do you have any thoughts about the order of parameters?

See inline comments for suggested updates.

robertdefilippi · 2022-04-26T13:12:46Z

@dbeatty10 This fell off my radar for a bit, but you make a good suggestion. I'll have a look when I get some time. Thanks!

…x_to_star

…ils into add_regex_to_star

robertdefilippi · 2022-04-27T13:49:36Z

I'm going to close this PR and reopen with a fresh branch.

robertdefilippi added 2 commits December 18, 2021 12:00

Add regex functionality

e2bd120

Update readme

d1cca68

robertdefilippi mentioned this pull request Dec 20, 2021

Cannot build docker for postgres integration test #344

Closed

joellabes added the pending label Jan 23, 2022

Update formatting

26a8b9f

Update tests

3d7a7d6

robertdefilippi added 6 commits January 27, 2022 22:06

Fix FROM in test macro

b44d2e6

Fix if else statement

b23df89

Fix control flow

e38940b

Convert to string

823fe2e

Fix byte error

b11f53f

Refactor tests and macro

78daa8a

Update readme

6979072

joellabes removed the pending label Feb 21, 2022

joellabes added the pending label Mar 22, 2022

Merge branch 'main' into next/minor

6785200

dbeatty10 reviewed Apr 26, 2022

View reviewed changes

dbeatty10 requested changes Apr 26, 2022

View reviewed changes

robertdefilippi added 9 commits April 26, 2022 20:38

Move regex input in signature

162791c

Fix merge conflicts

0b19e76

Fix merge conflicts

88758d3

Merge remote-tracking branch 'origin/add_regex_to_star' into add_rege…

fc739e4

…x_to_star

Merge remote-tracking branch 'origin/add_regex_to_star' into add_rege…

91a1a41

…x_to_star

Merge branch 'add_regex_to_star' of github.com:robertdefilippi/dbt-ut…

f327de3

…ils into add_regex_to_star

Adjust column logic

48a64d5

Adjust column logic

ee52bc6

Adjust column logic

7f0d708

robertdefilippi closed this Apr 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add regex to star #466

Add regex to star #466

robertdefilippi commented Dec 18, 2021 •

edited by dbeatty10

Loading

robertdefilippi commented Dec 18, 2021 •

edited

Loading

joellabes commented Jan 23, 2022

robertdefilippi commented Jan 24, 2022

joellabes commented Jan 24, 2022

robertdefilippi commented Jan 24, 2022

joellabes commented Jan 27, 2022

robertdefilippi commented Jan 27, 2022 •

edited

Loading

robertdefilippi commented Feb 6, 2022

joellabes commented Feb 9, 2022

joellabes commented Feb 21, 2022

robertdefilippi commented Feb 21, 2022

dbeatty10 Apr 26, 2022

dbeatty10 left a comment

robertdefilippi commented Apr 26, 2022

robertdefilippi commented Apr 27, 2022

Add regex to star #466

Add regex to star #466

Conversation

robertdefilippi commented Dec 18, 2021 • edited by dbeatty10 Loading

Description & motivation

Checklist

robertdefilippi commented Dec 18, 2021 • edited Loading

joellabes commented Jan 23, 2022

robertdefilippi commented Jan 24, 2022

joellabes commented Jan 24, 2022

robertdefilippi commented Jan 24, 2022

joellabes commented Jan 27, 2022

robertdefilippi commented Jan 27, 2022 • edited Loading

robertdefilippi commented Feb 6, 2022

joellabes commented Feb 9, 2022

joellabes commented Feb 21, 2022

robertdefilippi commented Feb 21, 2022

dbeatty10 Apr 26, 2022

Choose a reason for hiding this comment

dbeatty10 left a comment

Choose a reason for hiding this comment

robertdefilippi commented Apr 26, 2022

robertdefilippi commented Apr 27, 2022

robertdefilippi commented Dec 18, 2021 •

edited by dbeatty10

Loading

robertdefilippi commented Dec 18, 2021 •

edited

Loading

robertdefilippi commented Jan 27, 2022 •

edited

Loading