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

Dev Container environment #882

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ghjklw
Copy link

@ghjklw ghjklw commented Dec 17, 2024

Description

To facilitate working on this project, I began setting up a Dev Container / GitHub codespace environment.

This is a fairly basic setup at this point, but I believe it might be useful to others, and a good way to lower the threshold to contributing to the adapter.

Potential for improvement:

  • I struggled a bit to write a script to automate the setup of a databricks_demo project as described in CONTRIBUTING.md because the dbt-databricks adapter doesn't seem to be recognized by dbt when installed in editable mode.
  • The unit tests should work out of the box, but I haven't been able to test setting up the integration tests.
  • There is potential to implement a few more reocmmendations/best practices for environment configuration and setup to make the experience even smoother (for example install a couple of tools like databricks CLI or argcomplete, add relevant VS Code extensions or settings), but these might be better to handle as follow-up PR in order to discuss their relevance individually.

Could you please let me know if this is something you're interesting in before I put any more effort in it.

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.

Signed-off-by: Malo Jaffré <[email protected]>
@benc-db
Copy link
Collaborator

benc-db commented Dec 17, 2024

Awesome! This is a cool effort, and will review in the next day or so.

@benc-db
Copy link
Collaborator

benc-db commented Dec 17, 2024

I really need to update the contributing doc, but thank you for taking a pass at it :).

Copy link
Collaborator

@benc-db benc-db left a comment

Choose a reason for hiding this comment

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

See comments, but generally looks good.

.devcontainer/devcontainer.json Show resolved Hide resolved
.devcontainer/devcontainer.json Show resolved Hide resolved
"python.languageServer": "Pylance",
"ruff.fixAll": true,
"ruff.format.args": [
"--config=pyproject.toml"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wouldn't think this is necessary; I would think it would default to local pyproject.toml.

Copy link
Author

Choose a reason for hiding this comment

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

You're probably right, I will test and remove if unnecessary.

```
tox -e linter
```
This project uses [Ruff](https://docs.astral.sh/ruff/) and [mypy](https://www.mypy-lang.org/) for linting and static type checks.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you!

@benc-db
Copy link
Collaborator

benc-db commented Dec 18, 2024

Just add an entry in the Changelog (1.9.2) when you're ready for me to merge this. You might want to give a pointer to how to connect this up in docs/dbt-databricks-dev.md

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.

2 participants