-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
updated docs functionality to account for dataclasses refactor #1716
updated docs functionality to account for dataclasses refactor #1716
Conversation
Hi @drewbanin. I believe CLA stuff should be in order as well? I changed my primary github email to match the info on the CLA, but let me know if you have any additional issues. Thanks! |
Thanks @talagluck! I'm kicking off the test suite now. I do expect that there will be many tiny changes required to the unit/integration test suite - lmk if you have any questions about how to resolve those once the test results roll in :) |
core/dbt/source_config.py
Outdated
@@ -7,7 +7,7 @@ | |||
|
|||
class SourceConfig: | |||
AppendListFields = {'pre-hook', 'post-hook', 'tags'} | |||
ExtendDictFields = {'vars', 'column_types', 'quoting', 'persist_docs'} | |||
ExtendDictFields = {'vars', 'column_types', 'quoting', 'persist_docs', 'docs'} |
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.
Looks like this line failed for pep8 reasons:
core/dbt/source_config.py:10:80: E501 line too long (82 > 79 characters)
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 fixed.
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.
Great, thanks! Tests are running now
Hey @talagluck - you should be able to see a list of failed unit and integration tests now. There are a good number of them, but the fixes are all really tiny - you'll just need to add an empty Definitely let me know if you'd like a hand or some pointers! To run the unit / integration tests locally, check out our contributing guide |
Hi @drewbanin. I fixed a few of the tests and committed without thinking, so I'm waiting for the CI to complete again. I tried setting up an environment to run tests locally and got this error when trying to run
|
Hey @talagluck - just kicked off the tests again. Re: that error: I think you may need to run:
via the contributing guide. Let me know if you've already tried that! |
Thanks, @drewbanin! I did do that. I basically followed the instructions line by line until that point. |
huh! And you're still not able to get it working? I don't think you should need to do this, but maybe you can try running:
This should create the root user, which should allow the rest of the commands to succeed. Let me know if that fixes it - we might need to update the docs? |
Hi @drewbanin. I ran |
@drewbanin, I've updated some more tests based off of the results of the CI. I've been having a bunch of trouble getting my environment set up for proper testing, so I did it sort of blindly. Would you have some time today or tomorrow to pair on figuring out that devloop? Thanks! |
Sure thing @talagluck - feel free to drop me a line on Slack - happy to help you get this sorted. |
Hi @drewbanin, I've made some updates to the tests and a minor update the |
@talagluck should be running now! |
hey @talagluck - looks like the unit tests are passing! I can see some failures on the Postgres integration tests, and I bet there are other minor test tweaks to make around Snowflake/BigQuery/Redshift too. Let me know if there's anything I can help out with here! |
Thanks, @drewbanin! I was out last week and now I'm catching up on things, but I'm hoping to get back to this tomorrow. I might have a slight issue running the Postgres tests, and I'll let you know tomorrow if I can't figure anything out. |
Hi @drewbanin - I'm having some trouble getting a testing postgres environment to work on my machine, and also understanding the failing errors. Do you have a few minutes to pair today? |
Hi @drewbanin, I'm back from parental leave, and I'm looking to finish this up. I'm going to go to the #development channel of dbt slack for broader questions, but wanted to check in here to find out what branch I should rebase from/to at this stage. Thanks! |
hey @talagluck - welcome back! We're still on |
…ature/rebuilding-docs-color-on-refactor
Hi @drewbanin, apologies - this keeps getting away from us. Where do things stand now? What branch would it be best to work from at this point? Thanks! |
hey @talagluck - this can go out in our 0.16.0 release - you'll want to change the base branch to |
@talagluck I'm going to close this PR out as stale, but will happily re-open if you're interested in picking it back up! |
Thank you @drewbanin! I've been out on leave and just came back. Apologies for the delays on this. I'll let you know when we have the resources to jump back in! |
Following up on this PR: 1713
This code has been refactored to account for the new dbt dataclasses re-factor.