Skip to content

Commit b35ba94

Browse files
authored
Clarified documentation (#137)
1 parent f1f99cb commit b35ba94

File tree

1 file changed

+24
-22
lines changed

1 file changed

+24
-22
lines changed

CONTRIBUTING.md

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,40 +3,40 @@
33
## First Principles
44

55
We must use the [Databricks SDK for Python](https://databricks-sdk-py.readthedocs.io/) in this project. It is a toolkit for our project.
6-
If something doesn't naturally belong to the `WorkspaceClient`, it needs to go through a "mixin" process before it can be used with the SDK.
7-
Imagine the `WorkspaceClient` as the main control center, and the "mixin" process as a way to adapt other things to work with it.
6+
If something doesn't naturally belong to the `WorkspaceClient`, it must go through a "mixin" process before it can be used with the SDK.
7+
Imagine the `WorkspaceClient` as the main control center and the "mixin" process as a way to adapt other things to work with it.
88
You can find an example of how mixins are used with `StatementExecutionExt`. There's a specific example of how to make something
99
work with the WorkspaceClient using `StatementExecutionExt`. This example can help you understand how mixins work in practice.
1010

1111
## Code Organization
1212

13-
When you're writing code, make sure to divide it into two main parts: **Components for API Interaction** and **Components for Business Logic**.
14-
API Interaction should only deal with talking to external systems through APIs. They are usually integration tested and mocks are simpler.
13+
When writing code, divide it into two main parts: **Components for API Interaction** and **Components for Business Logic**.
14+
API Interaction should only deal with talking to external systems through APIs. They are usually integration-tested, and mocks are simpler.
1515
Business Logic handles the actual logic of your application, like calculations, data processing, and decision-making.
1616

1717
_Keep API components simple._ In the components responsible for API interactions, try to keep things as straightforward as possible.
18-
Don't overload them with lots of complex logic; instead, focus on making API calls and handling the data from those calls.
18+
Refrain from overloading them with complex logic; instead, focus on making API calls and handling the data from those calls.
1919

2020
_Inject Business Logic._ If you need to use business logic in your API-calling components, don't build it directly there.
2121
Instead, inject (or pass in) the business logic components into your API components. This way, you can keep your API components
2222
clean and flexible, while the business logic remains separate and reusable.
2323

24-
_Test your Business Logic._ It's essential to thoroughly test your business logic to ensure it works correctly. When writing
25-
unit tests, avoid making actual API calls - unit tests are executed for every pull request and **_take seconds to complete_**.
24+
_Test your Business Logic._ It's essential to test your business logic to ensure it works correctly and thoroughly. When writing
25+
unit tests, avoid making actual API calls - unit tests are executed for every pull request, and **_take seconds to complete_**.
2626
For calling any external services, including Databricks Connect, Databricks Platform, or even Apache Spark, unit tests have
27-
to use "mocks" or fake versions of the APIs to simulate their behavior. This makes it easier to test your code and catch any
27+
to use "mocks" or fake versions of the APIs to simulate their behavior. This makes testing your code more manageable and catching any
2828
issues without relying on external systems. Focus on testing the edge cases of the logic, especially the scenarios where
2929
things may fail. See [this example](https://github.com/databricks/databricks-sdk-py/pull/295) as a reference of an extensive
3030
unit test coverage suite and the clear difference between _unit tests_ and _integration tests_.
3131

3232
## Integration Testing Infrastructure
3333

34-
All new code additions must be accompanied by integration tests. Integration tests help us validate that various parts of
34+
Integration tests must accompany all new code additions. Integration tests help us validate that various parts of
3535
our application work correctly when they interact with each other or external systems. This practice ensures that our
3636
software _**functions as a cohesive whole**_. Integration tests run every night and take approximately 15 minutes
37-
for the whole test suite to complete.
37+
for the entire test suite to complete.
3838

39-
For integration tests, we encourage using predefined test infrastructure provided through environment variables.
39+
We encourage using predefined test infrastructure provided through environment variables for integration tests.
4040
These fixtures are set up in advance to simulate specific scenarios, making it easier to test different use cases. These
4141
predefined fixtures enhance test consistency and reliability and point to the real infrastructure used by integration
4242
testing. See [Unified Authentication Documentation](https://databricks-sdk-py.readthedocs.io/en/latest/authentication.html)
@@ -54,19 +54,19 @@ for the latest reference of environment variables related to authentication.
5454
- `TEST_DEFAULT_WAREHOUSE_ID`: This variable contains the identifier for the default warehouse used in testing. The value
5555
resembles a unique warehouse ID, like "49134b80d2...".
5656
- `TEST_INSTANCE_POOL_ID`: This environment variable stores the identifier for the instance pool used in testing.
57-
You must utilise existing instance pools as much as possible for the sake of cluster startup time and cost reduction.
57+
You must utilise existing instance pools as much as possible for cluster startup time and cost reduction.
5858
The value is a unique instance pool ID, like "0824-113319-...".
5959
- `TEST_LEGACY_TABLE_ACL_CLUSTER_ID`: This variable holds the identifier for the cluster used in testing legacy table
6060
access control. The value is a unique cluster ID, like "0824-161440-...".
6161
- `TEST_USER_ISOLATION_CLUSTER_ID`: This environment variable contains the identifier for the cluster used in testing
6262
user isolation. The value is a unique cluster ID, like "0825-164947-...".
6363

64-
We encourage you to leverage the extensive set of [pytest fixtures](https://docs.pytest.org/en/latest/explanation/fixtures.html#about-fixtures).
65-
These fixtures follow a consistent naming pattern, starting with "make_". These are functions that can be called multiple
64+
We'd like to encourage you to leverage the extensive set of [pytest fixtures](https://docs.pytest.org/en/latest/explanation/fixtures.html#about-fixtures).
65+
These fixtures follow a consistent naming pattern, starting with "make_". These functions can be called multiple
6666
times to _create and clean up objects as needed_ for your tests. Reusing these fixtures helps maintain clean and consistent
67-
test setups across the codebase. In cases where your tests require unique fixture setups, it's crucial to keep the wall
68-
clock time of fixture initialization under one second. Fast fixture initialization ensures that tests run quickly, reducing
69-
development cycle times and allowing for faster feedback during development.
67+
test setups across the codebase. In cases where your tests require unique fixture setups, keeping the wall
68+
clock time of fixture initialization under one second is crucial. Fast fixture initialization ensures that tests run quickly, reducing
69+
development cycle times and allowing for more immediate feedback during development.
7070

7171
```python
7272
from databricks.sdk.service.workspace import AclPermission
@@ -80,11 +80,11 @@ def test_secret_scope_acl(make_secret_scope, make_secret_scope_acl, make_group):
8080
Each integration test _must be debuggable within the free [IntelliJ IDEA (Community Edition)](https://www.jetbrains.com/idea/download)
8181
with the [Python plugin (Community Edition)](https://plugins.jetbrains.com/plugin/7322-python-community-edition). If it works within
8282
IntelliJ CE, then it would work in PyCharm. Debugging capabilities are essential for troubleshooting and diagnosing issues during
83-
development. Ensure that your test setup allows for easy debugging by following best practices.
83+
development. Please make sure that your test setup allows for easy debugging by following best practices.
8484

8585
![debugging tests](./examples/debugging-tests.gif)
8686

87-
By adhering to these guidelines, we ensure that our integration tests are robust, efficient, and easily maintainable. This,
87+
Adhering to these guidelines ensures that our integration tests are robust, efficient, and easily maintainable. This,
8888
in turn, contributes to the overall reliability and quality of our software.
8989

9090
Currently, VSCode IDE is not supported, as it does not offer interactive debugging single integration tests.
@@ -144,6 +144,8 @@ Here are the example steps to submit your first contribution:
144144
9. .. fix if any
145145
10. `git commit -a`. Make sure to enter meaningful commit message title.
146146
11. `git push origin FEATURENAME`
147-
12. go to GitHub UI and create PR. Alternatively, `gh pr create` (if you have [GitHub CLI](https://cli.github.com/) installed).
148-
Make sure to use meaningful pull request title, because it'll appear in the release notes.
149-
13. announce PR for the review
147+
12. Go to GitHub UI and create PR. Alternatively, `gh pr create` (if you have [GitHub CLI](https://cli.github.com/) installed).
148+
Use a meaningful pull request title because it'll appear in the release notes. Use `Resolves #NUMBER` in pull
149+
request description to [automatically link it](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests#linking-a-pull-request-to-an-issue)
150+
to an existing issue.
151+
14. announce PR for the review

0 commit comments

Comments
 (0)