Skip to content

Commit e5cbde8

Browse files
docs: Contribution doc updates (#181)
* No prompt s for add-apt * Script update * Improve contributing * Update PR template * Proofread * Proofread
1 parent 6cadd46 commit e5cbde8

5 files changed

Lines changed: 108 additions & 171 deletions

File tree

.github/pull_request_template.md

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
> [!NOTE]
2-
> Thank you for making change! Please consider filling this template for your pull request to improve quality of checkin message.
2+
> Thank you for making change! Please fill this template for your pull request to improve quality of check-in message.
33
44
> [!TIP]
55
> This repo uses [Conventional Commit conventions](https://www.conventionalcommits.org/en/v1.0.0/) - please try to rename your PR headline to match it.
66
7+
> [!WARNING]
8+
> Please ensure to read through this whole set of instructions, specially the `Test` section.
9+
710
# Why this change is needed
811

912
Describe what issue this change is trying to address.
@@ -22,4 +25,25 @@ Describe how the change works.
2225

2326
# Test
2427

25-
- What tests have been run? Please describe any verification steps you used.
28+
## Important: Non-Microsoft Employee contributors
29+
30+
If you are not a Microsoft employee with a `foo@microsoft.com` email, you will not be able to run CI as it runs in the `@microsoft.com` Fabric Tenant where you do not have access.
31+
32+
In order for your PR to be considered for review, you **must** attach a clear screenshot of the output of you running the following command successfully:
33+
34+
```bash
35+
npx nx run dbt-fabricspark:test --output-style=stream
36+
```
37+
38+
Here's an example of a successful run:
39+
40+
![A successful CI run locally](https://rakirahman.blob.core.windows.net/public/images/Misc/dbt-fabricspark-ci-run-success.png)
41+
42+
> ⚠️ Delete the above image and attach your own screenshot
43+
44+
To keep the quality of the repo high, if you **do not** attach a screenshot of successful local testing, your PR will be promptly closed.
45+
46+
## Microsoft Employee contributors
47+
48+
Your PR will be subjected to full regression suite via GitHub Action.
49+
It's highly recommended to run the tests locally so your contributions are promptly merged rather than failing in CI.

CONTRIBUTING.md

Lines changed: 73 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,107 +1,100 @@
1-
# Contributing to `dbt-fabricspark`
1+
# Contributing
22

3-
1. [About this document](#about-this-document)
4-
3. [Getting the code](#getting-the-code)
5-
5. [Running `dbt-fabricspark` in development](#running-dbt-fabricspark-in-development)
6-
6. [Testing](#testing)
7-
7. [Updating Docs](#updating-docs)
8-
7. [Submitting a Pull Request](#submitting-a-pull-request)
3+
[![Deep-dive Walkthrough](https://rakirahman.blob.core.windows.net/public/images/Misc/dbt-fabricspark-contrib.png)](https://rakirahman.blob.core.windows.net/public/videos/dbt-fabricspark-local-development-deep-dive.mp4)
94

10-
## About this document
11-
This document is a guide intended for folks interested in contributing to `dbt-fabricspark`. Below, we document the process by which members of the community should create issues and submit pull requests (PRs) in this repository. It is not intended as a guide for using `dbt-fabricspark`, and it assumes a certain level of familiarity with Python concepts such as virtualenvs, `pip`, Python modules, and so on. This guide assumes you are using macOS or Linux and are comfortable with the command line.
5+
If you're a windows user, we only use windows to get into WSL - everything is Linux from there.
126

13-
For those wishing to contribute we highly suggest reading the dbt-core's [contribution guide](https://github.com/dbt-labs/dbt-core/blob/HEAD/CONTRIBUTING.md) if you haven't already. Almost all of the information there is applicable to contributing here, too!
7+
Spark and Livy are notoriously hard to run on Windows.
8+
Therefore, the development environment we support is Linux, using [VSCode Devcontainer](https://code.visualstudio.com/docs/devcontainers/containers).
9+
This repo's CI runs the exact same devcontainer.
1410

15-
## Getting the code
11+
Therefore, if the tests pass locally, they are highly likely to pass in CI as well.
1612

17-
You will need `git` in order to download and modify the `dbt-fabricspark` source code. You can find directions [here](https://github.com/git-guides/install-git) on how to install `git`.
13+
## How to use, on a Linux machine
1814

19-
### External contributors
15+
1. Windows pre-reqs
2016

21-
If you are not a member of the `Microsoft` GitHub organization, you can contribute to `dbt-fabricspark` by forking the `dbt-fabricspark` repository. For a detailed overview on forking, check out the [GitHub docs on forking](https://help.github.com/en/articles/fork-a-repo). In short, you will need to:
17+
```powershell
18+
winget install -e --id Microsoft.VisualStudioCode
19+
```
2220

23-
1. fork the `dbt-fabricspark` repository
24-
2. clone your fork locally
25-
3. check out a new branch for your proposed changes
26-
4. push changes to your fork
27-
5. open a pull request against `microsoft/dbt-fabricspark` from your forked repository
21+
1. Get a fresh new WSL machine up:
2822

29-
### Microsoft Org contributors
23+
```powershell
24+
$GIT_ROOT = git rev-parse --show-toplevel
25+
& "$GIT_ROOT\contrib\bootstrap-dev-env.ps1"
26+
```
3027

31-
If you are a member of the `Microsoft` GitHub organization, you will have push access to the `dbt-fabricspark` repo. Rather than forking `dbt-fabricspark` to make your changes, just clone the repository, check out a new branch, and push directly to that branch.
28+
1. Clone the repo, and open VSCode in it:
3229

30+
```bash
31+
sudo mkdir -p /workspaces && sudo chmod 777 /workspaces && cd /workspaces
3332

34-
## Running `dbt-fabricspark` in development
33+
read -p "Enter your name (e.g. 'FirstName LastName'): " user_name
34+
read -p "Enter your email (e.g. 'your-alias@foo.com'): " user_email
35+
read -p "Enter your git fork (e.g. 'https://github.com/microsoft/dbt-fabricspark.git'): " git_fork_url
36+
read -p "Enter the existing branch to switch to: (e.g. 'main'): " branch_name
37+
38+
git config --global user.name "$user_name"
39+
git config --global user.email "$user_email"
40+
git clone "$git_fork_url"
3541

36-
### Installation
42+
cd /workspaces/dbt-fabricspark
43+
git checkout "$branch_name"
3744

38-
First make sure that you set up your `virtualenv` as described in [Setting up an environment](https://github.com/dbt-labs/dbt-core/blob/HEAD/CONTRIBUTING.md#setting-up-an-environment). Ensure you have the uv installed with `pip install uv` or via curl.
45+
code .
46+
```
3947

40-
Next, if this is first time installation of `dbt-fabricspark` with latest dependencies:
48+
1. Run the bootstrapper script, that installs all tools idempotently:
4149

42-
```sh
43-
uv pip install -e . --group dev
44-
```
50+
```bash
51+
GIT_ROOT=$(git rev-parse --show-toplevel)
52+
chmod +x ${GIT_ROOT}/contrib/bootstrap-dev-env.sh && ${GIT_ROOT}/contrib/bootstrap-dev-env.sh
53+
```
4554

46-
If have installed packages locally using uv pip install, make sure to run uv sync to update the uv.lock file from your environment.
47-
```sh
48-
uv pip install <package>
49-
```
55+
1. Launch the devcontainer:
5056

51-
When `dbt-fabricspark` is installed this way, any changes you make to the `dbt-fabricspark` source code will be reflected immediately in your next `dbt-fabricspark` run.
57+
```bash
58+
cd /workspaces/dbt-fabricspark
59+
HEX=$(printf '%s' "$(wslpath -w .)" | xxd -ps -c 256)
60+
code --folder-uri "vscode-remote://dev-container+${HEX}/workspaces/dbt-fabricspark"
61+
```
5262

53-
To confirm you have correct version of `dbt-core` installed please run `dbt --version` and `which dbt`.
63+
1. Install recommended developer tooling (optional):
5464

65+
```bash
66+
curl -fsSL https://gh.io/copilot-install | bash
67+
$HOME/.local/bin/copilot --yolo
68+
```
5569

56-
## Testing
70+
1. Login to github and ensure to authorize `Microsoft` if you're an employee (optional):
5771

58-
### Initial Setup
72+
```bash
73+
gh auth login
74+
```
5975

60-
`dbt-fabricspark` uses test credentials specified in a `test.env` file in the root of the repository. This `test.env` file is git-ignored, but please be _extra_ careful to never check in credentials or other sensitive information when developing. To create your `test.env` file, copy the provided example file, then supply your relevant credentials.
76+
1. Create a `test.env` file with the two Fabric workspace IDs and display names — you need `Contributor` on both. The functional test suite uses **two** workspaces:
6177

62-
```
63-
cp test.env.example test.env
64-
$EDITOR test.env
65-
```
78+
- **Workspace 1 (`WORKSPACE_ID_1` / `WORKSPACE_NAME_1`)** — the *primary* workspace where dbt models are materialized during tests.
79+
- **Workspace 2 (`WORKSPACE_ID_2` / `WORKSPACE_NAME_2`)** — the *secondary* workspace that hosts a seeded fixture lakehouse, used to verify cross-workspace 4-part naming end-to-end.
6680

67-
### Test commands
68-
There are a few methods for running tests locally.
81+
```bash
82+
GIT_ROOT=$(git rev-parse --show-toplevel)
83+
read -p "Enter Fabric workspace 1 ID: " ws1_id
84+
read -p "Enter Fabric workspace 1 display name: " ws1_name
85+
read -p "Enter Fabric workspace 2 ID: " ws2_id
86+
read -p "Enter Fabric workspace 2 display name: " ws2_name
87+
{
88+
echo "WORKSPACE_ID_1=${ws1_id}"
89+
echo "WORKSPACE_NAME_1=${ws1_name}"
90+
echo "WORKSPACE_ID_2=${ws2_id}"
91+
echo "WORKSPACE_NAME_2=${ws2_name}"
92+
} > "${GIT_ROOT}/test.env"
93+
```
6994

70-
#### `tox`
71-
`tox` takes care of managing Python virtualenvs and installing dependencies in order to run tests. You can also run tests in parallel, for example you can run unit tests for Python 3.8, Python 3.9, and `flake8` checks in parallel with `tox -p`. Also, you can run unit tests for specific python versions with `tox -e <env>`. The configuration of these tests are located in `tox.ini`.
95+
1. All build and tests should now run green:
7296

73-
#### `pytest`
74-
Finally, you can also run a specific test or group of tests using `pytest` directly. With a Python virtualenv active and dev dependencies installed you can do things like:
75-
76-
```sh
77-
# run all functional tests
78-
uv run pytest --profile az_cli tests/functional/
79-
# run specific functional tests
80-
uv run pytest --profile az_cli tests/functional/adapter/basic/*
81-
# run all unit tests in a file
82-
uv run pytest tests/unit/test_adapter.py
83-
# run a specific unit test
84-
uv run pytest test/unit/test_adapter.py::TestSparkAdapter::test_profile_with_database
85-
```
86-
## Updating Docs
87-
88-
Many changes will require and update to the `dbt-fabricspark` docs here are some useful resources.
89-
90-
- Docs are [here](https://docs.getdbt.com/).
91-
- The docs repo for making changes is located [here]( https://github.com/dbt-labs/docs.getdbt.com).
92-
- The changes made are likely to impact one or both of [Fabric Spark Profile](https://docs.getdbt.com/reference/warehouse-profiles/fabricspark-profile), or [Saprk Configs](https://docs.getdbt.com/reference/resource-configs/spark-configs).
93-
- We ask every community member who makes a user-facing change to open an issue or PR regarding doc changes.
94-
95-
## Adding CHANGELOG Entry
96-
97-
Changelogs are managed manually for now. As you raise a PR, provide the changes made in your commits.
98-
99-
## Submitting a Pull Request
100-
101-
Microsoft provides a CI environment to test changes to the `dbt-fabricspark` adapter, and periodic checks against the development version of `dbt-core` through Github Actions.
102-
103-
A `dbt-fabricspark` maintainer will review your PR. They may suggest code revision for style or clarity, or request that you add unit or functional test(s). These are good things! We believe that, with a little bit of help, anyone can contribute high-quality code.
104-
105-
Once all requests and answers have been answered the `dbt-fabricspark` maintainer can trigger CI testing.
106-
107-
Once all tests are passing and your PR has been approved, a `dbt-fabricspark` maintainer will merge your changes into the active development branch. And that's it! Happy developing :tada:
97+
```bash
98+
npx nx run dbt-fabricspark:build
99+
npx nx run dbt-fabricspark:test --output-style=stream
100+
```

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ The `dbt-fabricspark` package contains all of the code enabling dbt to work with
5151
- [Install dbt](https://docs.getdbt.com/docs/installation)
5252
- Read the [introduction](https://docs.getdbt.com/docs/introduction/) and [viewpoint](https://docs.getdbt.com/docs/about/viewpoint/)
5353

54+
> To contribute to this adapter codebase, see [CONTRIBUTING.md](CONTRIBUTING.md).
55+
5456
### Installation
5557

5658
```bash

contrib/README.md

Lines changed: 0 additions & 86 deletions
This file was deleted.

contrib/bootstrap-dev-env.sh

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ command -v gh &>/dev/null || PACKAGES="$PACKAGES gh"
2222
if ! [ -x "$(command -v docker)" ]; then
2323
echo "docker is not installed on your devbox, installing..."
2424
curl -fsSL https://download.docker.com/linux/ubuntu/gpg | sudo apt-key add -
25-
sudo add-apt-repository "deb [arch=amd64] https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable"
25+
sudo add-apt-repository -y "deb [arch=amd64] https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable"
2626
sudo apt-get update -q
2727
sudo apt-get install -y apt-transport-https ca-certificates curl
2828
sudo apt-get install -y --allow-downgrades docker-ce="$DOCKER_VERSION" docker-ce-cli="$DOCKER_VERSION" containerd.io
@@ -40,10 +40,14 @@ if grep -q "$ACR_URL" ~/.docker/config.json 2>/dev/null; then
4040
if [ -n "$ACR_PASSWORD" ]; then
4141
docker_password="$ACR_PASSWORD"
4242
else
43-
read -sp "Enter Docker Admin password for ${ACR_URL}: " docker_password
43+
read -sp "If you are a Microsoft Employee and you plan on contributing to the Devcontainer image, please ping @mdrrahman and enter Docker Admin password for ${ACR_URL} - otherwise, leave blank and press [ENTER]: " docker_password
4444
echo
4545
fi
46-
echo "$docker_password" | docker login "$ACR_URL" --username "$ACR_NAME" --password-stdin
46+
if [ -z "$docker_password" ]; then
47+
echo "You left the password empty, skipping docker login"
48+
else
49+
echo "$docker_password" | docker login "$ACR_URL" --username "$ACR_NAME" --password-stdin
50+
fi
4751
fi
4852

4953
export PATH=$(echo "$PATH" | tr ':' '\n' | grep -v "/mnt/c" | tr '\n' ':' | sed 's/:$//')

0 commit comments

Comments
 (0)