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

Copy code from dbt core #13

Merged
merged 27 commits into from
Jan 11, 2024
Merged

Copy code from dbt core #13

merged 27 commits into from
Jan 11, 2024

Conversation

MichelleArk
Copy link
Collaborator

@MichelleArk MichelleArk commented Jan 4, 2024

Resolves: dbt-labs/dbt-core#10

Acceptance Criteria

Follow-up work:

  • point first party adapters to github main of dbt-common
    • will be done as part of ongoing 1p adapter migrations instead

@MichelleArk MichelleArk force-pushed the copy-code-from-dbt-core branch from 0c380be to e5f7b08 Compare January 4, 2024 15:32
@MichelleArk MichelleArk force-pushed the copy-code-from-dbt-core branch from e5f7b08 to 243ac3c Compare January 4, 2024 17:01
@emmyoop emmyoop force-pushed the copy-code-from-dbt-core branch from 3eb228a to 9c69956 Compare January 6, 2024 02:14
@MichelleArk MichelleArk force-pushed the copy-code-from-dbt-core branch from 21a08ef to 009e971 Compare January 10, 2024 15:42
@MichelleArk MichelleArk marked this pull request as ready for review January 10, 2024 16:09
Copy link
Contributor

@mikealfare mikealfare left a comment

Choose a reason for hiding this comment

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

I left a few comments. Most were around the idea that we might want to move some of this to dbt-adapter if it's specific to an adapter. In particular, if the primary functionality lives there (e.g. connection), then any associated functionality should also live there (or we need to move the primary back to dbt-common).

dbt_common/contracts/connection.py Outdated Show resolved Hide resolved
dbt_common/contracts/constraints.py Outdated Show resolved Hide resolved
dbt_common/exceptions/__init__.py Show resolved Hide resolved
dbt_common/exceptions/cache.py Outdated Show resolved Hide resolved
dbt_common/exceptions/connection.py Show resolved Hide resolved
dbt_common/utils/connection.py Outdated Show resolved Hide resolved
dbt_common/utils/jinja.py Outdated Show resolved Hide resolved
tests/unit/__init__.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@colin-rogers-dbt colin-rogers-dbt left a comment

Choose a reason for hiding this comment

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

just nitpicking otherwise LGTM

dbt_common/events/README.md Outdated Show resolved Hide resolved
dbt_common/events/README.md Outdated Show resolved Hide resolved
tests/unit/test_core_dbt_utils.py Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
@mikealfare
Copy link
Contributor

fwiw, I'm able to build dbt-adapter off of this branch. I'm working on updating references where I guessed and then running unit tests.

@mikealfare
Copy link
Contributor

Update, I migrated all unit tests to dbt-adapter that seemed relevant to the package. I was able to build dbt-adapter off of this branch and get all 100 unit tests to pass after updating references and limiting scope to dbt-adapter concerns.

Copy link
Contributor

@mikealfare mikealfare left a comment

Choose a reason for hiding this comment

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

Approving assuming the cache change goes through as discussed separately.

@MichelleArk MichelleArk merged commit 18fd40f into main Jan 11, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

need to be able to choose which models get deployed
4 participants