Skip to content

Commit

Permalink
doc: Document our connectors QA checks (#35324)
Browse files Browse the repository at this point in the history
  • Loading branch information
alafanechere authored Feb 19, 2024
1 parent f2efd27 commit 87d792e
Show file tree
Hide file tree
Showing 13 changed files with 198 additions and 62 deletions.
6 changes: 5 additions & 1 deletion airbyte-ci/connectors/connectors_qa/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,11 @@ This package uses two local dependencies:

To add a new QA check, you have to create add new class in one of the `checks` module. This class must inherit from `models.Check` and implement the `_run` method. Then, you need to add an instance of this class to the `ENABLED_CHECKS` list of the module.

**Please run the `generate-doumentation` command to update the documentation with the new check and commit it in your PR.**
**Please run the `generate-documentation` command to update the documentation with the new check and commit it in your PR.**:
```bash
# From airbyte repo root
connectors-qa generate-documentation docs/contributing-to-airbyte/resources/qa-checks.md
```

### Running tests

Expand Down
128 changes: 73 additions & 55 deletions airbyte-ci/connectors/connectors_qa/poetry.lock

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions airbyte-ci/connectors/connectors_qa/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ pytest = "^8.0.0"
pytest-mock = "^3.12.0"
mypy = "^1.8.0"
types-toml = "^0.10.8.7"
pytest-asyncio = "^0.23.5"
gitpython = "^3.1.42"

[build-system]
requires = ["poetry-core"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class AssetsCheck(Check):

class CheckConnectorIconIsAvailable(AssetsCheck):
name = "Connectors must have an icon"
description = "Each connector must have an icon available in at the root of the connector code directory. It must be an SVG file named `icon.svg`."
description = "Each connector must have an icon available in at the root of the connector code directory. It must be an SVG file named `icon.svg` and must be a square."
requires_metadata = False

def _run(self, connector: Connector) -> CheckResult:
Expand All @@ -27,6 +27,7 @@ def _run(self, connector: Connector) -> CheckResult:
passed=False,
message="Icon file is not named 'icon.svg'",
)
# TODO check that the icon is a square
return self.create_check_result(connector=connector, passed=True, message="Icon file exists")


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class DocumentationCheck(Check):

class CheckMigrationGuide(DocumentationCheck):
name = "Breaking changes must be accompanied by a migration guide"
description = "When a breaking change is introduced we check that a migration guide is available. It should be stored under `./docs/integrations/<connector-type>s/<connector-name>-migrations.md`.\nThis document should contain a section for each breaking change, in order of the version descending. It must explain users which action to take to migrate to the new version."
description = "When a breaking change is introduced, we check that a migration guide is available. It should be stored under `./docs/integrations/<connector-type>s/<connector-name>-migrations.md`.\nThis document should contain a section for each breaking change, in order of the version descending. It must explain users which action to take to migrate to the new version."

def _run(self, connector: Connector) -> CheckResult:
breaking_changes = get(connector.metadata, "releases.breakingChanges")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def _run(self, connector: Connector) -> CheckResult:

class CheckConnectorLicenseMatchInPyproject(PackagingCheck):
name = f"Connector license in {consts.METADATA_FILE_NAME} and {consts.PYPROJECT_FILE_NAME} file must match"
description = f"Connectors license in {consts.METADATA_FILE_NAME} and {consts.PYPROJECT_FILE_NAME} file must match. This is to ensure that all connectors are consistently licensed"
description = f"Connectors license in {consts.METADATA_FILE_NAME} and {consts.PYPROJECT_FILE_NAME} file must match. This is to ensure that all connectors are consistently licensed."
applies_to_connector_languages = [
ConnectorLanguage.PYTHON,
ConnectorLanguage.LOW_CODE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ They are by no mean replacing the need for a manual review of the connector code
*Applies to the following connector languages: {{ ', '.join(check.applies_to_connector_languages) }}*

{{ check.description }}
{% endfor %}
{%- endfor %}
{% endfor %}
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Copyright (c) 2023 Airbyte, Inc., all rights reserved.


from pathlib import Path

import git
import pytest
from asyncclick.testing import CliRunner
from connectors_qa.cli import generate_documentation

DOCUMENTATION_FILE_PATH_IN_AIRBYTE_REPO = Path("docs/contributing-to-airbyte/resources/qa-checks.md")

@pytest.fixture
def airbyte_repo():
return git.Repo(search_parent_directories=True)

@pytest.mark.asyncio
async def test_generated_qa_checks_documentation_is_up_to_date(airbyte_repo, tmp_path):
# Arrange
current_doc = (airbyte_repo.working_dir / DOCUMENTATION_FILE_PATH_IN_AIRBYTE_REPO).read_text()
newly_generated_doc_path = tmp_path / "qa-checks.md"

# Act
await CliRunner().invoke(generate_documentation, [str(tmp_path / "qa-checks.md")], catch_exceptions=False)

# Assert
suggested_command = f"connectors-qa generate-documentation {DOCUMENTATION_FILE_PATH_IN_AIRBYTE_REPO}"
assert newly_generated_doc_path.read_text() == current_doc, f"The generated documentation is not up to date. Please run `{suggested_command}` and commit the changes."
4 changes: 2 additions & 2 deletions docs/contributing-to-airbyte/change-cdk-connector.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ Make sure you're working on an issue had been already triaged to not have your c
3. Code the change
4. Write a unit test for each custom function you added or changed
5. Ensure all tests, including connector acceptance tests, pass
6. Update the `metadata.yaml` and `Dockerfile` version following the [guidelines](./resources/pull-requests-handbook.md#semantic-versioning-for-connectors)
6. Update the `metadata.yaml` following the [guidelines](./resources/pull-requests-handbook.md#semantic-versioning-for-connectors)
7. Update the changelog entry in documentation in `docs/integrations/<connector-name>.md`
8. Make sure your contribution passes our [QA checks](./resources/qa-checks.md)

A comment will automatically be added to your PR with a checklist containing the necessary steps to complete your contribution and get it merged.

:::info
There is a README file inside each connector folder containing instructions to run that connector's tests locally.
Expand Down
81 changes: 81 additions & 0 deletions docs/contributing-to-airbyte/resources/qa-checks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# Airbyte connectors QA checks

This document is listing all the static-analysis checks that are performed on the Airbyte connectors.
These checks are running in our CI/CD pipeline and are used to ensure a connector is following the best practices and is respecting the Airbyte standards.
Meeting these standards means that the connector will be able to be safely integrated into the Airbyte platform and released to registries (DockerHub, Pypi etc.).
You can consider these checks as a set of guidelines to follow when developing a connector.
They are by no mean replacing the need for a manual review of the connector codebase and the implementation of good test suites.


## 📄 Documentation

### Breaking changes must be accompanied by a migration guide
*Applies to the following connector languages: java, low-code, python*

When a breaking change is introduced, we check that a migration guide is available. It should be stored under `./docs/integrations/<connector-type>s/<connector-name>-migrations.md`.
This document should contain a section for each breaking change, in order of the version descending. It must explain users which action to take to migrate to the new version.
### Connectors must have user facing documentation
*Applies to the following connector languages: java, low-code, python*

The user facing connector documentation should be stored under `./docs/integrations/<connector-type>s/<connector-name>.md`.
### Connectors documentation follows our guidelines
*Applies to the following connector languages: java, low-code, python*

The user facing connector documentation should follow the guidelines defined in the [documentation standards](https://hackmd.io/Bz75cgATSbm7DjrAqgl4rw).
### Connectors must have a changelog entry for each version
*Applies to the following connector languages: java, low-code, python*

Each new version of a connector must have a changelog entry defined in the user facing documentation in `./docs/integrations/<connector-type>s/<connector-name>.md`.

## 📝 Metadata

### Connectors must have valid metadata.yaml file
*Applies to the following connector languages: java, low-code, python*

Connectors must have a `metadata.yaml` file at the root of their directory. This file is used to build our connector registry. Its structure must follow our metadata schema. Field values are also validated. This is to ensure that all connectors have the required metadata fields and that the metadata is valid. More details in this [documentation](https://docs.airbyte.com/connector-development/connector-metadata-file).

## 📦 Packaging

### Connectors must use Poetry for dependency management
*Applies to the following connector languages: python, low-code*

Connectors must use [Poetry](https://python-poetry.org/) for dependency management. This is to ensure that all connectors use a dependency management tool which locks dependencies and ensures reproducible installs.
### Connectors must be licensed under MIT or Elv2
*Applies to the following connector languages: java, low-code, python*

Connectors must be licensed under the MIT or Elv2 license. This is to ensure that all connectors are licensed under a permissive license. More details in our [License FAQ](https://docs.airbyte.com/developer-guides/licenses/license-faq).
### Connector license in metadata.yaml and pyproject.toml file must match
*Applies to the following connector languages: python, low-code*

Connectors license in metadata.yaml and pyproject.toml file must match. This is to ensure that all connectors are consistently licensed.
### Connector version must follow Semantic Versioning
*Applies to the following connector languages: java, low-code, python*

Connector version must follow the Semantic Versioning scheme. This is to ensure that all connectors follow a consistent versioning scheme. Refer to our [Semantic Versioning for Connectors](https://docs.airbyte.com/contributing-to-airbyte/#semantic-versioning-for-connectors) for more details.
### Connector version in metadata.yaml and pyproject.toml file must match
*Applies to the following connector languages: python, low-code*

Connector version in metadata.yaml and pyproject.toml file must match. This is to ensure that connector release is consistent.
### Python connectors must have PyPi publishing enabled
*Applies to the following connector languages: python, low-code*

Python connectors must have [PyPi](https://pypi.org/) publishing enabled in their `metadata.yaml` file. This is declared by setting `remoteRegistries.pypi.enabled` to `true` in metadata.yaml. This is to ensure that all connectors can be published to PyPi and can be used in `airbyte-lib`.

## 💼 Assets

### Connectors must have an icon
*Applies to the following connector languages: java, low-code, python*

Each connector must have an icon available in at the root of the connector code directory. It must be an SVG file named `icon.svg` and must be a square.

## 🔒 Security

### Connectors must use HTTPS only
*Applies to the following connector languages: java, low-code, python*

Connectors must use HTTPS only when making requests to external services.
### Python connectors must not use a Dockerfile and must declare their base image in metadata.yaml file
*Applies to the following connector languages: python, low-code*

Connectors must use our Python connector base image (`docker.io/airbyte/python-connector-base`), declared through the `connectorBuildOptions.baseImage` in their `metadata.yaml`.
This is to ensure that all connectors use a base image which is maintained and has security updates.
1 change: 1 addition & 0 deletions docs/contributing-to-airbyte/submit-new-connector.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ This will enable our team to make sure your contribution does not overlap with e
3. Code the change
4. Ensure all tests pass. For connectors, this includes acceptance tests as well.
5. Update documentation in `docs/integrations/<connector-name>.md`
6. Make sure your contribution passes our [QA checks](./resources/qa-checks.md)


#### Open a pull request
Expand Down
1 change: 1 addition & 0 deletions docusaurus/sidebars.js
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ const contributeToAirbyte = {
"contributing-to-airbyte/resources/pull-requests-handbook",
"contributing-to-airbyte/resources/code-style",
"contributing-to-airbyte/resources/code-formatting",
"contributing-to-airbyte/resources/qa-checks",
"contributing-to-airbyte/resources/developing-locally",
"contributing-to-airbyte/resources/developing-on-docker",
],
Expand Down

0 comments on commit 87d792e

Please sign in to comment.