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

metadata: fix tests not running, fix failing non-running tests, fix validate base images exist #33127

Merged
merged 23 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
349721c
add debugging info
erohmensing Dec 5, 2023
88debfe
fix invalid data fixture and assert that our fixtures have contents
erohmensing Dec 5, 2023
a5821f6
fix debugging expected errors
erohmensing Dec 5, 2023
d18fef1
add 2 tests and fix one which didn't represent what the filename indi…
erohmensing Dec 5, 2023
89bc218
fix incorrect arg name
erohmensing Dec 5, 2023
75a09b1
fix incorrect property name
erohmensing Dec 5, 2023
2250e1f
add more clarity
erohmensing Dec 5, 2023
e695919
clarify even more
erohmensing Dec 5, 2023
2bc38c0
use correct path to retrieve base image
erohmensing Dec 5, 2023
483da70
fix stub
erohmensing Dec 5, 2023
0a67bc0
add retries to the regular docker hub checks. why the hell not
erohmensing Dec 5, 2023
367ca98
fix stub for base images
erohmensing Dec 5, 2023
fe76b12
fix base images for stubbing
erohmensing Dec 5, 2023
0cc9dd3
mock dockerhub for valid files check
erohmensing Dec 5, 2023
2f60529
move base image tests to validate, not upload. add new one for if the…
erohmensing Dec 5, 2023
a9b95cf
better error info if no GCS creds, and patch dockerhub in test_valida…
erohmensing Dec 5, 2023
6e305de
Automated Commit - Formatting Changes
erohmensing Dec 5, 2023
75590dc
remove redundant comment
erohmensing Dec 6, 2023
eafc72e
fix error messages for if validation succeeds when it shouldnt have
erohmensing Dec 6, 2023
2a836f0
refactor the checking it failed correctly
erohmensing Dec 6, 2023
b513577
use pytest.fail
erohmensing Dec 6, 2023
534c7e2
use better mock versions
erohmensing Dec 6, 2023
2ecbbc7
assume shas are valid by default
erohmensing Dec 6, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,11 @@ def upload_metadata_to_gcs(bucket_name: str, metadata_file_path: Path, validator
if metadata is None:
raise ValueError(f"Metadata file {metadata_file_path} is invalid for uploading: {error}")

service_account_info = json.loads(os.environ.get("GCS_CREDENTIALS"))
gcs_creds = os.environ.get("GCS_CREDENTIALS")
if not gcs_creds:
raise ValueError("Please set the GCS_CREDENTIALS env var.")

service_account_info = json.loads(gcs_creds)
credentials = service_account.Credentials.from_service_account_info(service_account_info)
storage_client = storage.Client(credentials=credentials)
bucket = storage_client.bucket(bucket_name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def validate_metadata_images_in_dockerhub(

print(f"Checking that the following images are on dockerhub: {images_to_check}")
for image, version in images_to_check:
if not is_image_on_docker_hub(image, version):
if not is_image_on_docker_hub(image, version, retries=3):
return False, f"Image {image}:{version} does not exist in DockerHub"

return True, None
Expand Down Expand Up @@ -151,7 +151,7 @@ def validate_metadata_base_images_in_dockerhub(
) -> ValidationResult:
metadata_definition_dict = metadata_definition.dict()

image_address = get(metadata_definition_dict, "data.connectorOptions.baseImage")
image_address = get(metadata_definition_dict, "data.connectorBuildOptions.baseImage")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

❗ this fixes the behavior of the validator - previously it wasn't checking the right tag. This means that we have not been running this validation, and should probably do a run on all connectors on this branch/after this is merged to make sure that no connectors currently reference invalid base images.

Copy link
Contributor

Choose a reason for hiding this comment

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

nice catch! I don't think running full validation is necessary: if the value of this field is incorrect the connector build will fail, so we'd probably catch this error on publish failure anyway.

if image_address is None:
return True, None

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,45 @@

def list_all_paths_in_fixture_directory(folder_name: str) -> List[str]:
file_path = os.path.join(os.path.dirname(__file__), folder_name)

# If folder_name has subdirectories, os.walk will return a list of tuples,
# one for folder_name and one for each of its subdirectories.
fixture_files = []
for root, dirs, files in os.walk(file_path):
return [os.path.join(root, file_name) for file_name in files]
fixture_files.extend(os.path.join(root, file_name) for file_name in files)
return fixture_files
Comment on lines +10 to +16
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes an issue where cleaning up our fixtures by adding subdirectories actually led to no files being found for the fixture - we weren't looking in the subdirectories for files because we early returned after grabbing the (non-nested) files from the directory itself.



@pytest.fixture(scope="session")
def valid_metadata_yaml_files() -> List[str]:
return list_all_paths_in_fixture_directory("metadata_validate/valid")
files = list_all_paths_in_fixture_directory("metadata_validate/valid")
if not files:
pytest.fail("No files found in metadata_validate/valid")
return files


@pytest.fixture(scope="session")
def invalid_metadata_yaml_files() -> List[str]:
return list_all_paths_in_fixture_directory("metadata_validate/invalid")
files = list_all_paths_in_fixture_directory("metadata_validate/invalid")
if not files:
pytest.fail("No files found in metadata_validate/invalid")
return files


@pytest.fixture(scope="session")
def valid_metadata_upload_files() -> List[str]:
return list_all_paths_in_fixture_directory("metadata_upload/valid")
files = list_all_paths_in_fixture_directory("metadata_upload/valid")
if not files:
pytest.fail("No files found in metadata_upload/valid")
return files


@pytest.fixture(scope="session")
def invalid_metadata_upload_files() -> List[str]:
return list_all_paths_in_fixture_directory("metadata_upload/invalid")
files = list_all_paths_in_fixture_directory("metadata_upload/invalid")
if not files:
pytest.fail("No files found in metadata_upload/invalid")
return files


@pytest.fixture(scope="session")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ data:
releaseStage: alpha
releases:
breakingChanges:
6.0.0: # tag does not exist
0.0.0: # tag does not exist
upgradeDeadline: 2023-08-22
message: "This version made a change."
documentationUrl: https://docs.airbyte.com/integrations/sources/onesignal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ data:
cloud:
enabled: true
dockerRepository: airbyte/exists-3
dockerImageTag: 6.6.6 # tag does not exist
dockerImageTag: 99.99.99 # tag does not exist
oss:
enabled: true
dockerRepository: airbyte/exists-4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ data:
connectorType: source
dockerRepository: airbyte/exists-1
githubIssueLabel: source-alloydb-strict-encrypt
dockerImageTag: 6.6.6 # tag does not exist
dockerImageTag: 99.99.99 # tag does not exist
documentationUrl: https://docs.airbyte.com/integrations/sources/existingsource
connectorSubtype: database
releaseStage: generally_available
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ data:
normalizationConfig:
normalizationIntegrationType: postgres
normalizationRepository: airbyte/exists-2
dockerImageTag: 6.6.6 # tag does not exist
normalizationTag: 99.99.99 # tag does not exist
registries:
cloud:
enabled: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ data:
oss:
enabled: true
dockerRepository: airbyte/exists-4
dockerImageTag: 6.6.6 # tag does not exist
dockerImageTag: 99.99.99 # tag does not exist
tags:
- language:java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ data:
connectorType: source
dockerRepository: airbyte/exists-1
githubIssueLabel: source-alloydb-strict-encrypt
dockerImageTag: 6.6.6 # tag does not exist
dockerImageTag: 99.99.99 # tag does not exist
documentationUrl: https://docs.airbyte.com/integrations/sources/existingsource
connectorSubtype: database
releaseStage: generally_available
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ data:
hosts:
- zopim.com
connectorBuildOptions:
baseImage: docker.io/airbyte/python-connector-base:1.1.0@sha256:bd98f6505c6764b1b5f99d3aedc23dfc9e9af631a62533f60eb32b1d3dbab20c
baseImage: docker.io/airbyte/base-repo-exists:1.1.0@sha256:bd98f6505c6764b1b5f99d3aedc23dfc9e9af631a62533f60eb32b1d3dbab20c
connectorSubtype: api
connectorType: source
definitionId: 40d24d0f-b8f9-4fe0-9e6c-b06c0f3f45e4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ data:
hosts:
- zopim.com
connectorBuildOptions:
baseImage: docker.io/airbyte/python-connector-base:1.1.0@sha256:doesnotexists
baseImage: docker.io/airbyte/base-repo-exists:1.1.0@sha256:MISSINGSHA
connectorSubtype: api
connectorType: source
definitionId: 40d24d0f-b8f9-4fe0-9e6c-b06c0f3f45e4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ data:
hosts:
- zopim.com
connectorBuildOptions:
baseImage: docker.io/airbyte/python-connector-base:does-not-exists@sha256:bd98f6505c6764b1b5f99d3aedc23dfc9e9af631a62533f60eb32b1d3dbab20c
baseImage: docker.io/airbyte/base-repo-exists:99.99.99@sha256:bd98f6505c6764b1b5f99d3aedc23dfc9e9af631a62533f60eb32b1d3dbab20c
connectorSubtype: api
connectorType: source
definitionId: 40d24d0f-b8f9-4fe0-9e6c-b06c0f3f45e4
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
metadataSpecVersion: 1.0
data:
name: AlloyDB for PostgreSQL
definitionId: 1fa90628-2b9e-11ed-a261-0242ac120002
connectorType: source
dockerRepository: airbyte/image-exists-1
githubIssueLabel: source-alloydb-strict-encrypt
dockerImageTag: 0.0.1
documentationUrl: https://docs.airbyte.com/integrations/sources/alloydb
connectorSubtype: database
releaseStage: generally_available
license: MIT
releasestests/fixtures/metadata_validate/invalid/metadata_breaking_change_versions_under_releases.yml:
2.1.3:
message: "This version changes the connector’s authentication method from `ApiKey` to `oAuth`, per the [API guide](https://amazon-sqs.com/api/someguide)."
upgradeDeadline: 2023-08-22
tags:
- language:java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
metadataSpecVersion: 1.0
data:
name: AlloyDB for PostgreSQL
definitionId: 1fa90628-2b9e-11ed-a261-0242ac120002
connectorType: source
dockerRepository: airbyte/image-exists-1
githubIssueLabel: source-alloydb-strict-encrypt
dockerImageTag: 0.0.1
documentationUrl: https://docs.airbyte.com/integrations/sources/alloydb
connectorSubtype: database
releaseStage: generally_available
license: MIT
breakingChanges:
2.1.3:
message: "This version changes the connector’s authentication method from `ApiKey` to `oAuth`, per the [API guide](https://amazon-sqs.com/api/someguide)."
upgradeDeadline: 2023-08-22
tags:
- language:java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
data:
allowedHosts:
hosts:
- "*.googleapis.com"
connectorBuildOptions:
baseImage: docker.io/airbyte/base-repo-exists:1.1.0
connectorSubtype: file
connectorType: source
definitionId: 71607ba1-c0ac-4799-8049-7f4b90dd50f7
dockerImageTag: 0.3.7
dockerRepository: airbyte/source-google-sheets
githubIssueLabel: source-google-sheets
icon: google-sheets.svg
license: Elv2
name: Google Sheets
registries:
cloud:
enabled: true
oss:
enabled: true
releaseStage: generally_available
documentationUrl: https://docs.airbyte.com/integrations/sources/google-sheets
tags:
- language:python
ab_internal:
sl: 300
ql: 400
supportLevel: certified
metadataSpecVersion: "1.0"
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ data:
enabled: true
releaseStage: alpha
releases:
1.0.0:
upgradeDeadline: 2023-08-22
message: "This version made a change."
breakingChanges:
1.0.0:
upgradeDeadline: 2023-08-22
message: "This version made a change."
documentationUrl: https://docs.airbyte.com/integrations/sources/onesignal
tags:
- language:python
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ data:
hosts:
- "*.googleapis.com"
connectorBuildOptions:
baseImage: airbyte/python-connector-base:1.0.0
baseImage: docker.io/airbyte/base-repo-exists:1.1.0@sha256:bd98f6505c6764b1b5f99d3aedc23dfc9e9af631a62533f60eb32b1d3dbab20c
connectorSubtype: file
connectorType: source
definitionId: 71607ba1-c0ac-4799-8049-7f4b90dd50f7
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,16 @@
from metadata_service.gcs_upload import MetadataUploadInfo, UploadedFile
from metadata_service.validators.metadata_validator import ValidatorOptions
from pydantic import BaseModel, ValidationError, error_wrappers
from test_gcs_upload import stub_is_image_on_docker_hub


# TEST VALIDATE COMMAND
def test_valid_metadata_yaml_files(valid_metadata_yaml_files, tmp_path):
def test_valid_metadata_yaml_files(mocker, valid_metadata_yaml_files, tmp_path):
runner = CliRunner()

# Mock dockerhub for base image checks
mocker.patch("metadata_service.validators.metadata_validator.is_image_on_docker_hub", side_effect=stub_is_image_on_docker_hub)

assert len(valid_metadata_yaml_files) > 0, "No files found"

for file_path in valid_metadata_yaml_files:
Expand All @@ -30,13 +34,13 @@ def test_invalid_metadata_yaml_files(invalid_metadata_yaml_files, tmp_path):

for file_path in invalid_metadata_yaml_files:
result = runner.invoke(commands.validate, [file_path, str(tmp_path)])
assert result.exit_code != 0, f"Validation succeeded (when it shouldve failed) for {file_path}"
assert result.exit_code != 0, f"Validation succeeded (when it should have failed) for {file_path}"


def test_metadata_file_not_found_fails(tmp_path):
runner = CliRunner()
result = runner.invoke(commands.validate, ["non_existent_file.yaml", str(tmp_path)])
assert result.exit_code != 0, "Validation succeeded (when it shouldve failed) for non_existent_file.yaml"
assert result.exit_code != 0, "Validation succeeded (when it should have failed) for non_existent_file.yaml"


def test_docs_path_not_found_fails(valid_metadata_yaml_files):
Expand All @@ -45,7 +49,7 @@ def test_docs_path_not_found_fails(valid_metadata_yaml_files):
assert len(valid_metadata_yaml_files) > 0, "No files found"

result = runner.invoke(commands.validate, [valid_metadata_yaml_files[0], "non_existent_docs_path"])
assert result.exit_code != 0, "Validation succeeded (when it shouldve failed) for non_existent_docs_path"
assert result.exit_code != 0, "Validation succeeded (when it should have failed) for non_existent_docs_path"


def mock_metadata_upload_info(
Expand Down
Loading
Loading