Running dbt schema builder tool section added #38
Conversation
README.rst
Outdated
| Getting Started | ||
| ----------------- | ||
| If you want to run dbt-schema-builder locally, clone this repository on your local system. | ||
| You need a python virtual environment to run this, use the following commands to create and activate python 3.6.12 virtual environment |
There was a problem hiding this comment.
Not everyone uses pyenv to manage their virtual environments, it might make sense to call out here that this example uses pyenv, or just say "create a Python 3.6 virtual environment using your favorite tool"
README.rst
Outdated
|
|
||
| Next, you will run following commands | ||
| :: | ||
| $ pip install -U pip-tools |
There was a problem hiding this comment.
Users shouldn't need to install pip-tools or do a pip-compile to get started, and doing so might upgrade dependencies to a non-working state. They should just be able to do a make upgrade to get the pinned dependencies that are already checked in.
There was a problem hiding this comment.
FWIW, this was a required step for both jazib and I. It was some weird compatibility issue with an old version of pip-tools---the makefile would install the old broken pip-tools which would break any/all subsequent lines in the make recipe. However, I think codifying this one-time fix is not appropriate, so we should remove these steps like you recommend.
There was a problem hiding this comment.
I will make the changes in this readme file generic as @bmedx suggested, I will write these troubleshooting and warehouse-transform related steps in internal wiki
README.rst
Outdated
| :: | ||
| $ pip install . | ||
|
|
||
| Now you will be all set to run dbt-schema-builder tool locally, cd into warehouse-transforms project directory in which you want to run dbt-schema-builder tool |
There was a problem hiding this comment.
This is a public repo, so not everyone will have access to warehouse transforms. I think you can generalize this to "cd into the dbt project directory in which you want to run..."
README.rst
Outdated
| :: | ||
| $ cd warehouse-transforms/projects/automated/applications | ||
|
|
||
| Replace the --profiles-dir path |
There was a problem hiding this comment.
The flags on this command will all be different for people who aren't using the edX repos, so we should generalize this example as well.
|
@bmedx Hey! I have made the changes and tried to make it generic. |
jazibhumayun
force-pushed
the
jazibhumayun/gettingstarted-added
branch
from
e697ceb
to
ca5626c
Compare
|
Sorry, I just noticed we test multiple versions of python (3.6 and 3.8). How come we're recommending to use a specific version (3.6) in the docs? |
README.rst
Outdated
| Inside dbt-schema-builder folder, run the following to update the versions in all of the local requirements files | ||
| :: | ||
| $ cd dbt-schema-builder | ||
| $ make upgrade |
There was a problem hiding this comment.
Also, I don't think we need to tell people to run make upgrade if they're just trying to run schema builder. That's a maintenance task left for the maintainers (us).
There was a problem hiding this comment.
Yeah agreed. I think this should be make requirements, I always confuse the two.
There was a problem hiding this comment.
Sorry, I just noticed we test multiple versions of python (3.6 and 3.8). How come we're recommending to use a specific version (3.6) in the docs?
I saw 3.6 version from this doc https://openedx.atlassian.net/wiki/spaces/DE/pages/1121059960/dbt+Upgrade+Checklist, if I works on multiple versions of python for sure I can change the text form here.
Actually this PR isn't part of any story I was just trying to document the steps that I did in order to run dbt-schema-builder (in a similar way that warehouse-transform repository has getting started steps), I wasn't aware of this being a public repository
There was a problem hiding this comment.
oh yes it should be make requirements
There was a problem hiding this comment.
Thanks for catching all these things in the review, my intent was to document steps for internal team members, but its good to have it here as well from perspective of external users, thanks for detailed review.
jazibhumayun
changed the title
Description: Describe in a couple of sentences what this PR adds
JIRA: Link to JIRA ticket
Dependencies: dependencies on other outstanding PRs, issues, etc.
Merge deadline: List merge deadline (if any)
Installation instructions: List any non-trivial installation
instructions.
Testing instructions:
Reviewers:
Merge checklist:
Post merge:
finished.
Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.