-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ GitPython = "^3.1.29" | |
pydantic = "^1.9" | ||
PyGithub = "^1.58.0" | ||
rich = "^13.0.0" | ||
pydash = "^7.0.4" | ||
pydash = "^6.0.2" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why downgrade? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm aligning to Aligning to |
||
google-cloud-storage = "^2.8.0" | ||
ci-credentials = {path = "../ci_credentials"} | ||
pandas = "^2.0.3" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
# Connectors QA | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two questions here:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 😄 . |
||
|
||
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 | ||
``` |
Large diffs are not rendered by default.
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",] |
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()] |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.