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

Migrating to pyproject.toml and Hatch #853

Merged
merged 16 commits into from
Nov 26, 2024
Merged

Migrating to pyproject.toml and Hatch #853

merged 16 commits into from
Nov 26, 2024

Conversation

benc-db
Copy link
Collaborator

@benc-db benc-db commented Nov 22, 2024

Description

Begin using Hatch and pyproject.toml to manage environments/builds/testing. This matches the direction that dbt Labs has been moving. I have added instructions on how to get set up for development. Reviewer can you please follow the instructions and see if you hit any snags?

I will add PR comments to explain files

Checklist

  • 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 to the "dbt-databricks next" section.

- name: Get http path from environment
run: python .github/workflows/build_cluster_http_path.py
shell: sh
- name: Install tox

- name: Install Hatch
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Like Tox, installing Hatch is sufficient to install everything else

build:
name: build packages
- name: Run Unit Tests
run: hatch run -v +py=${{ matrix.python-version }} test:unit
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in the test environment we have a matrix for the supported python versions. This is how we request a particular entry in the matrix.

run: |
dbt --version
- name: Verify distributions
run: hatch run verify:check-all
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This does the same checks as before, but I'm no longer separating build and verify as separate tasks, just separate steps.

@@ -1 +1 @@
version: str = "1.9.0b1"
version = "1.9.0b1"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Had to drop the type info here to satisfy Hatch

@@ -109,7 +109,7 @@
@dataclass
class DatabricksConfig(AdapterConfig):
file_format: str = "delta"
table_format: TableFormat = TableFormat.DEFAULT
table_format: str = TableFormat.DEFAULT
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

on 3.12, mypy starts complaining that these enums actually inherit str, so their values are str.

@@ -764,7 +764,7 @@ class RelationAPIBase(ABC, Generic[DatabricksRelationConfig]):
For the most part, these are just namespaces to group related methods together.
"""

relation_type: ClassVar[DatabricksRelationType]
relation_type: ClassVar[str]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same str, enum issue as above.

@@ -224,7 +223,9 @@ def compile(self, path: str) -> PythonJobDetails:
if access_control_list:
job_spec["access_control_list"] = access_control_list

return PythonJobDetails(self.run_name, job_spec, additional_job_config)
return PythonJobDetails(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For some reason, mypy was complaining that this init was giving too many args when it was a dataclass. I have no idea what causes that, but switching to a pydantic model fixes the complaint.

"Programming Language :: Python :: 3.11",
"Programming Language :: Python :: 3.12",
]
dependencies = [
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These dependencies are the ones required to build the package. The default env dependencies specified below are what we want installed when we're working in IDE/using the CLI.

Copy link
Collaborator

Choose a reason for hiding this comment

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

are the dependencies exactly same as previous version? If not, shall we bump up the version

@@ -13,9 +13,9 @@ def query_relation_type(project, relation: BaseRelation) -> Optional[str]:
fetch="one",
)[0]
if table_type == "STREAMING_TABLE":
return DatabricksRelationType.StreamingTable.value
return DatabricksRelationType.StreamingTable
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same str, enum issue as above

@@ -1,8 +1,8 @@
from typing import Any, Callable
from unittest.mock import Mock
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

during this process I realized we were needlessly installing mock, as it has been a part of Python since 3.3.

alexguo-db
alexguo-db previously approved these changes Nov 26, 2024
@@ -34,6 +34,7 @@
- Fix behavior flag use in init of DatabricksAdapter (thanks @VersusFacit!) ([836](https://github.com/databricks/dbt-databricks/pull/836))
- Restrict pydantic to V1 per dbt Labs' request ([843](https://github.com/databricks/dbt-databricks/pull/843))
- Switching to Ruff for formatting and linting ([847](https://github.com/databricks/dbt-databricks/pull/847))
- Switching to Hatch and pyproject.toml for project config ([853](https://github.com/databricks/dbt-databricks/pull/853))
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you have to bump the minor version of this change/migration?

Copy link
Collaborator Author

@benc-db benc-db Nov 26, 2024

Choose a reason for hiding this comment

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

I would but this version hasn't been released yet.

@benc-db benc-db merged commit b95a04a into main Nov 26, 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.

4 participants