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.

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

Validate project names in interactive dbt init #4536

Merged
merged 3 commits into from
Jan 21, 2022
Merged

Validate project names in interactive dbt init #4536

merged 3 commits into from
Jan 21, 2022

Conversation

amirkdv
Copy link
Contributor

@amirkdv amirkdv commented Dec 28, 2021

resolves #4490

Description

This PR adds project name validation to dbt init.

  • workflow: ask the user to provide a valid project name until they do.
  • new integration tests
  • supported scenarios:
    • dbt init
    • dbt init -s
    • dbt init [name]
    • dbt init [name] -s

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change

Copy link
Contributor

@ChenyuLInx ChenyuLInx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amirkdv Thanks for contributing this!! Love the thoughtful functions and tests!

CHANGELOG.md Outdated Show resolved Hide resolved
test/integration/040_init_test/test_init.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@amirkdv amirkdv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @ChenyuLInx for the comments! I applied the suggested change and responded to your question. Let me know if I misunderstood the scenario you were describing!

test/integration/040_init_test/test_init.py Outdated Show resolved Hide resolved
Copy link
Contributor

@ChenyuLInx ChenyuLInx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me!

test/integration/040_init_test/test_init.py Outdated Show resolved Hide resolved
@ChenyuLInx
Copy link
Contributor

@amirkdv seems like there's a conflict blocking us from merging it, can you resolve it whenever you have a chance?

amirkdv and others added 3 commits January 21, 2022 17:09
- workflow: ask the user to provide a valid project name until they do.
- new integration tests
- supported scenarios:
  - dbt init
  - dbt init -s
  - dbt init [name]
  - dbt init [name] -s
@amirkdv
Copy link
Contributor Author

amirkdv commented Jan 21, 2022

@amirkdv seems like there's a conflict blocking us from merging it, can you resolve it whenever you have a chance?

I rebased and force-pushed to my branch. I think there was a file name change in main (test/integration/040_init_test/ -> test/integration/040_init_tests/) that git was unhappy about auto-resolving.

@ChenyuLInx
Copy link
Contributor

@amirkdv seems like there's a conflict blocking us from merging it, can you resolve it whenever you have a chance?

I rebased and force-pushed to my branch. I think there was a file name change in main (test/integration/040_init_test/ -> test/integration/040_init_tests/) that git was unhappy about auto-resolving.

Awesome!! Thanks!! I started the checks!

@ChenyuLInx ChenyuLInx merged commit 8791313 into dbt-labs:main Jan 21, 2022
@leahwicz leahwicz added the backport 1.0.latest Tag for PR to be backported to the 1.0.latest branch label Jan 27, 2022
@ChenyuLInx ChenyuLInx added backport 1.0.latest Tag for PR to be backported to the 1.0.latest branch and removed backport 1.0.latest Tag for PR to be backported to the 1.0.latest branch labels Feb 2, 2022
@leahwicz leahwicz added backport 1.0.latest Tag for PR to be backported to the 1.0.latest branch and removed backport 1.0.latest Tag for PR to be backported to the 1.0.latest branch labels Feb 2, 2022
ChenyuLInx added a commit that referenced this pull request Feb 3, 2022
* Validate project names in interactive dbt init

- workflow: ask the user to provide a valid project name until they do.
- new integration tests
- supported scenarios:
  - dbt init
  - dbt init -s
  - dbt init [name]
  - dbt init [name] -s

* Update Changelog.md

* Add full URLs to CHANGELOG.md

Co-authored-by: Chenyu Li <[email protected]>

Co-authored-by: Chenyu Li <[email protected]>
ChenyuLInx added a commit that referenced this pull request Feb 3, 2022
* Validate project names in interactive dbt init

- workflow: ask the user to provide a valid project name until they do.
- new integration tests
- supported scenarios:
  - dbt init
  - dbt init -s
  - dbt init [name]
  - dbt init [name] -s

* Update Changelog.md

* Add full URLs to CHANGELOG.md

Co-authored-by: Chenyu Li <[email protected]>

Co-authored-by: Chenyu Li <[email protected]>

Co-authored-by: Amir Kadivar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.0.latest Tag for PR to be backported to the 1.0.latest branch cla:yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-61] [Bug] interactive dbt init allows illegal project names
4 participants