-
Notifications
You must be signed in to change notification settings - Fork 163
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.
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
implement choice of delimiter for seed files #1122
implement choice of delimiter for seed files #1122
Conversation
Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: salim_moulouel.
|
028e9c2
to
e31a1f3
Compare
Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA. In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, please reach out through a comment on this PR. CLA has not been signed by users: @salimmoulouel |
e31a1f3
to
4934313
Compare
Thank you for opening this @salimmoulouel ! Please do the following:
|
Looking good so far @salimmoulouel 👍 A key thing we'll need is a functional test to make sure the change works. Thankfully, we already have a relevant test defined here -- we just need to inherit and run it within dbt-bigquery! Here's how to do it:
from dbt.tests.adapter.simple_seed.test_seed import BaseSeedWithUniqueDelimiter
class TestBigQuerySeedWithUniqueDelimiter(BaseSeedWithUniqueDelimiter):
pass Here's instructions how to run the test locally to make sure it breaks before your change but works once it is included. |
when i use
i get the following error
to get arround this problem i inherited the TestSimpleSeedConfigs and i modified the delimiter of the csv used in the seed, i get an error before applying change and it disappears after the changes |
Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com>
Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com>
Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com>
Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com>
@dbeatty10 i didn't get any news since my last commit, is the pull request okay ? |
Hope you're doing well! I wanted to chat about some merge requests I've submitted to dbt. It's been over Two months since I sent them in, and I haven't heard a peep back. Any chance you could give me the lowdown on what's up with them? I get that running an open-source project can be hectic, and I totally respect the hustle. But it's a bit frustrating not knowing where my contributions stand. I've put some real effort into these requests and would love to see them get some love. If there's anything I need to tweak or if my requests aren't quite hitting the mark, I'm all ears. Just looking for some clarity so I can get back in the game! Thanks a bunch for your time, and looking forward to hearing from you soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a quick tweak to the change log and this is good to merge
@salimmoulouel Sorry for the delay LGTM, can you just update the code formatting? (just need to run |
I went ahead and applied the formatting with the following, and it auto-merged once it passed all the CI checks 👍 ✅ gh pr checkout 1122
pre-commit run --all-files --show-diff-on-failure
git status .
git add dbt/adapters/bigquery/impl.py
git commit -m 'Apply formatting from pre-commit'
git push |
@dbeatty10 I see this code in At least one person is awaiting it: https://getdbt.slack.com/archives/CBSQTAPLG/p1724405788161219 |
* implement choice of delimiter for seed files * adding change log entry * implementation of test of TestBigQuerySeedWithUniqueDelimiter * Update dbt/adapters/bigquery/impl.py Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> * Update dbt/include/bigquery/macros/materializations/seed.sql Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> * Update dbt/include/bigquery/macros/materializations/seed.sql Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> * Update dbt/adapters/bigquery/impl.py Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> * Update .changes/unreleased/Fixes-20240226-233024.yaml --------- Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> Co-authored-by: Colin Rogers <111200756+colin-rogers-dbt@users.noreply.github.com> (cherry picked from commit 0d37ba6)
* implement choice of delimiter for seed files * adding change log entry * implementation of test of TestBigQuerySeedWithUniqueDelimiter * Update dbt/adapters/bigquery/impl.py Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> * Update dbt/include/bigquery/macros/materializations/seed.sql Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> * Update dbt/include/bigquery/macros/materializations/seed.sql Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> * Update dbt/adapters/bigquery/impl.py Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> * Update .changes/unreleased/Fixes-20240226-233024.yaml --------- Co-authored-by: Doug Beatty <44704949+dbeatty10@users.noreply.github.com> Co-authored-by: Colin Rogers <111200756+colin-rogers-dbt@users.noreply.github.com> (cherry picked from commit 0d37ba6) Co-authored-by: salimmoulouel <36620917+salimmoulouel@users.noreply.github.com>
resolves #1119
Problem
CSV seed work only with ',' delimiter.
Solution
This PR solves the problems by adding parameter of delimiter to the macro which build the table from the csv file
Checklist