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

[QA checks] tolerate connector without dockerfile #30467

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
21 changes: 13 additions & 8 deletions airbyte-ci/connectors/connector_ops/connector_ops/qa_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import sys
from pathlib import Path
from typing import Iterable, Optional, Set, Tuple
from typing import Callable, Iterable, Optional, Set, Tuple

from connector_ops.utils import Connector, ConnectorLanguage
from pydash.objects import get
Expand All @@ -14,7 +14,7 @@
def check_migration_guide(connector: Connector) -> bool:
"""Check if a migration guide is available for the connector if a breaking change was introduced."""

breaking_changes = get(connector.metadata, f"releases.breakingChanges")
breaking_changes = get(connector.metadata, "releases.breakingChanges")
if not breaking_changes:
return True

Expand All @@ -27,7 +27,7 @@ def check_migration_guide(connector: Connector) -> bool:

# Check that the migration guide begins with # {connector name} Migration Guide
expected_title = f"# {connector.name_from_metadata} Migration Guide"
expected_version_header_start = f"## Upgrading to "
expected_version_header_start = "## Upgrading to "
with open(migration_guide_file_path) as f:
first_line = f.readline().strip()
if not first_line == expected_title:
Expand Down Expand Up @@ -55,7 +55,7 @@ def check_migration_guide(connector: Connector) -> bool:

if ordered_breaking_changes != ordered_heading_versions:
print(f"Migration guide file for {connector.name} has incorrect version headings.")
print(f"Check for missing, extra, or misordered headings, or headers with typos.")
print("Check for missing, extra, or misordered headings, or headers with typos.")
print(f"Expected headings: {ordered_expected_headings}")
return False

Expand Down Expand Up @@ -242,7 +242,7 @@ def check_metadata_version_matches_dockerfile_label(connector: Connector) -> boo
return version_in_dockerfile == connector.version


QA_CHECKS = [
DEFAULT_QA_CHECKS = (
check_documentation_file_exists,
check_migration_guide,
# Disabling the following check because it's likely to not pass on a lot of connectors.
Expand All @@ -254,8 +254,13 @@ def check_metadata_version_matches_dockerfile_label(connector: Connector) -> boo
# https://github.com/airbytehq/airbyte/issues/21606
check_connector_https_url_only,
check_connector_has_no_critical_vulnerabilities,
check_metadata_version_matches_dockerfile_label,
]
)


def get_qa_checks_to_run(connector: Connector) -> Tuple[Callable]:
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

if connector.has_dockerfile:
return DEFAULT_QA_CHECKS + (check_metadata_version_matches_dockerfile_label,)
return DEFAULT_QA_CHECKS


def remove_strict_encrypt_suffix(connector_technical_name: str) -> str:
Expand Down Expand Up @@ -289,7 +294,7 @@ def run_qa_checks():
connector_technical_name = remove_strict_encrypt_suffix(connector_technical_name)
connector = Connector(connector_technical_name)
print(f"Running QA checks for {connector_technical_name}:{connector.version}")
qa_check_results = {qa_check.__name__: qa_check(connector) for qa_check in QA_CHECKS}
qa_check_results = {qa_check.__name__: qa_check(connector) for qa_check in get_qa_checks_to_run(connector)}
if not all(qa_check_results.values()):
print(f"QA checks failed for {connector_technical_name}:{connector.version}:")
for check_name, check_result in qa_check_results.items():
Expand Down
17 changes: 10 additions & 7 deletions airbyte-ci/connectors/connector_ops/connector_ops/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,10 @@ def icon_path(self) -> Path:
def code_directory(self) -> Path:
return Path(f"./airbyte-integrations/connectors/{self.technical_name}")

@property
def has_dockerfile(self) -> bool:
return (self.code_directory / "Dockerfile").is_file()

@property
def metadata_file_path(self) -> Path:
return self.code_directory / METADATA_FILE_NAME
Expand All @@ -321,20 +325,19 @@ def language(self) -> ConnectorLanguage:
return None

@property
def version(self) -> str:
def version(self) -> Optional[str]:
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this one optional? I get becuase self.version_in_dockerfile_label could now be None - but shouldn't we error out if we can't retrieve a version for the connector?

if self.metadata is None:
return self.version_in_dockerfile_label
return self.metadata["dockerImageTag"]

@property
def version_in_dockerfile_label(self) -> Optional[str]:
try:
with open(self.code_directory / "Dockerfile") as f:
for line in f:
if "io.airbyte.version" in line:
return line.split("=")[1].strip()
except FileNotFoundError:
if not self.has_dockerfile:
return None
with open(self.code_directory / "Dockerfile") as f:
for line in f:
if "io.airbyte.version" in line:
return line.split("=")[1].strip()
raise ConnectorVersionNotFound(
"""
Could not find the connector version from its Dockerfile.
Expand Down
Loading
Loading