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

Revamp QA checks into a battery included package #35322

Merged
merged 1 commit into from
Feb 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
879 changes: 399 additions & 480 deletions airbyte-ci/connectors/connector_ops/poetry.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion airbyte-ci/connectors/connector_ops/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ GitPython = "^3.1.29"
pydantic = "^1.9"
Copy link
Collaborator

Choose a reason for hiding this comment

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

What holds our back to switch to the pydantic 2.0.0? Just curious?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know, nothing I'm aware of, but it's a different package / project than the current one :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't yet know the full surface area of us using Pydantic. If we're using 1.* everywhere, should we treat 2.0+ upgrade as a separate lane of work, or do you mostly expect things to work smoothly with such an upgrade?

Is is drop-in compatible syntax-wise?

If we're using it in the CDK itself and in airbyte-ci — I think airbyte-ci could be our guinea pig for such a migration? /cc @bazarnov

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the only package that still demands the 1.* version is CAT, and yet, let the airbyte-ci be the guide indeed.

PyGithub = "^1.58.0"
rich = "^13.0.0"
pydash = "^7.0.4"
pydash = "^6.0.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why downgrade?

Copy link
Contributor Author

@alafanechere alafanechere Feb 16, 2024

Choose a reason for hiding this comment

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

connectors_qa depends on two other internal packages which are using pydash but on different version:

  • medata_service/lib uses v6
  • connector_ops uses v7

I'm aligning to metadata_service/lib version because this package is also a dependency of metadata_service/orchestrator, which is also declaring a dependency on pydash.

Aligning to v6 makes me modifying a single package (connector_ops) instead of 2 (metadata_service/lib / metadata_service/orchestrator). A part from that I don't think there's a good reason to stay on v6.

google-cloud-storage = "^2.8.0"
ci-credentials = {path = "../ci_credentials"}
pandas = "^2.0.3"
Expand Down
96 changes: 96 additions & 0 deletions airbyte-ci/connectors/connectors_qa/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Connectors QA
Copy link
Contributor

Choose a reason for hiding this comment

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

Two questions here:

  1. Does connectors_qa have to live in this particular directory, or is it just logical spot for it in the monorepo? I.e. could we just put it in the root of the repository instead?
  2. If it's fully independent of the rest of connector_ops stuff (don't think so), is there a good overall readme and list of directories and what project is where? We have many things — registry service, airbyte-ci, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@natikgadzhi I put it there because it's a package used in the CI context. But we could put it at the root of the repo, I don't mind 😄 .
It's dependent on a couple of other internal packages. I will explicitly list the dependency and why they exists in this README.


This package has two main purposes:
* Running QA checks on connectors.
* Generating the QA checks documentation that are run on connectors.



## Usage

### Install

```bash
pipx install .
```

This will make `connectors-qa` available in your `PATH`.


Feel free to run `connectors-qa --help` to see the available commands and options.


### Examples

#### Running QA checks on one or more connectors:

```bash
# This command must run from the root of the Airbyte repo
connectors-qa run --name=source-faker --name=source-google-sheets
```
#### Running QA checks on all connectors:

```bash
# This command must run from the root of the Airbyte repo
connectors-qa run --connector-directory=airbyte-integrations/connectors
```

#### Running QA checks on all connectors and generating a JSON report:

```bash
### Generating documentation for QA checks:
connectors-qa run --connector-directory=airbyte-integrations/connectors --report-path=qa_report.json
```

#### Running only specific QA checks on one or more connectors:

```bash
connectors-qa run --name=source-faker --name=source-google-sheets --check=CheckConnectorIconIsAvailable --check=CheckConnectorUsesPythonBaseImage
```

#### Running only specific QA checks on all connectors:

```bash
connectors-qa run --connector-directory=airbyte-integrations/connectors --check=CheckConnectorIconIsAvailable --check=CheckConnectorUsesPythonBaseImage
```

#### Generating documentation for QA checks:

```bash
connectors-qa generate-documentation qa_checks.md
```

## Development

```bash
poetry install
```

### Dependencies
This package uses two local dependencies:
* [`connector_ops`](https://github.com/airbytehq/airbyte/blob/master/airbyte-ci/connectors/connector_ops): To interact with the `Connector` object.
* [`metadata_service/lib`]((https://github.com/airbytehq/airbyte/blob/master/airbyte-ci/connectors/metadata_service/lib)): To validate the metadata of the connectors.

### Adding a new QA check

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.**

### Running tests

```bash
poe test
```

### Running type checks

```bash
poe type_check
```

### Running the linter

```bash
poe lint
```
2,457 changes: 2,457 additions & 0 deletions airbyte-ci/connectors/connectors_qa/poetry.lock

Large diffs are not rendered by default.

43 changes: 43 additions & 0 deletions airbyte-ci/connectors/connectors_qa/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
[tool.poetry]
name = "connectors-qa"
version = "1.0.0"
description = "A package to run QA checks on Airbyte connectors, generate reports and documentation."
authors = ["Airbyte <contact@airbyte.io>"]
readme = "README.md"
packages = [
{ include = "connectors_qa", from = "src" },
]
[tool.poetry.dependencies]
python = "^3.10"
airbyte-connectors-base-images = {path = "../base_images", develop = false}
connector-ops = {path = "../connector_ops", develop = false}
metadata-service = {path = "../metadata_service/lib", develop = false}
pydash = "^6.0.2"
jinja2 = "^3.1.3"
toml = "^0.10.2"
asyncclick = "^8.1.7.1"
asyncer = "^0.0.4"

[tool.poetry.scripts]
connectors-qa = "connectors_qa.cli:connectors_qa"

[tool.poetry.group.dev.dependencies]
ruff = "^0.2.1"
pytest = "^8.0.0"
pytest-mock = "^3.12.0"
mypy = "^1.8.0"
types-toml = "^0.10.8.7"

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"

[tool.poe.tasks]
test = "pytest tests"
type_check = "mypy src --disallow-untyped-defs"
lint = "ruff check src"

[tool.airbyte_ci]
extra_poetry_groups = ["dev"]
poe_tasks = ["type_check", "lint", "test"]
required_environment_variables = ["DOCKER_HUB_USERNAME", "DOCKER_HUB_PASSWORD",]
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Copyright (c) 2023 Airbyte, Inc., all rights reserved.
from .assets import ENABLED_CHECKS as ASSETS_CHECKS
from .metadata import ENABLED_CHECKS as METADATA_CORRECTNESS_CHECKS
from .security import ENABLED_CHECKS as SECURITY_CHECKS
from .packaging import ENABLED_CHECKS as PACKAGING_CHECKS
from .documentation import ENABLED_CHECKS as DOCUMENTATION_CHECKS

ENABLED_CHECKS = (
DOCUMENTATION_CHECKS
+ METADATA_CORRECTNESS_CHECKS
+ PACKAGING_CHECKS
+ ASSETS_CHECKS
+ SECURITY_CHECKS
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Copyright (c) 2023 Airbyte, Inc., all rights reserved.


from connector_ops.utils import Connector # type: ignore
from connectors_qa.models import Check, CheckCategory, CheckResult


class AssetsCheck(Check):
category = CheckCategory.ASSETS


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`."
requires_metadata = False

def _run(self, connector: Connector) -> CheckResult:
if not connector.icon_path or not connector.icon_path.exists():
return self.create_check_result(
connector=connector,
passed=False,
message="Icon file is missing. Please create an icon file at the root of the connector code directory.",
)
if not connector.icon_path.name == "icon.svg":
return self.create_check_result(
connector=connector,
passed=False,
message="Icon file is not named 'icon.svg'",
)
return self.create_check_result(connector=connector, passed=True, message="Icon file exists")


ENABLED_CHECKS = [CheckConnectorIconIsAvailable()]
Loading
Loading