From a7bb2806b482c7b697bb8dfa88a6658d50aaa3ac Mon Sep 17 00:00:00 2001 From: erohmensing Date: Wed, 15 May 2024 18:44:25 -0500 Subject: [PATCH 1/6] use icon file path before moving metadata file to a tempfile --- .../lib/metadata_service/commands.py | 5 +++++ .../lib/metadata_service/gcs_upload.py | 13 ++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/airbyte-ci/connectors/metadata_service/lib/metadata_service/commands.py b/airbyte-ci/connectors/metadata_service/lib/metadata_service/commands.py index 12604b08baa37..2652debfd17e2 100644 --- a/airbyte-ci/connectors/metadata_service/lib/metadata_service/commands.py +++ b/airbyte-ci/connectors/metadata_service/lib/metadata_service/commands.py @@ -17,6 +17,11 @@ def log_metadata_upload_info(metadata_upload_info: MetadataUploadInfo): click.secho( f"The {file.description} file for {metadata_upload_info.metadata_file_path} was uploaded to {file.blob_id}.", color="green" ) + else: + click.secho( + f"The {file.description} file for {metadata_upload_info.metadata_file_path} was not uploaded. Reason: {file.blob_id}", + color="yellow", + ) @click.group(help="Airbyte Metadata Service top-level command group.") diff --git a/airbyte-ci/connectors/metadata_service/lib/metadata_service/gcs_upload.py b/airbyte-ci/connectors/metadata_service/lib/metadata_service/gcs_upload.py index e128178a1c59c..0a2a32d42cbff 100644 --- a/airbyte-ci/connectors/metadata_service/lib/metadata_service/gcs_upload.py +++ b/airbyte-ci/connectors/metadata_service/lib/metadata_service/gcs_upload.py @@ -149,12 +149,11 @@ def _version_upload(metadata: ConnectorMetadataDefinitionV0, bucket: storage.buc return upload_file_if_changed(metadata_file_path, bucket, version_path, disable_cache=True) -def _icon_upload(metadata: ConnectorMetadataDefinitionV0, bucket: storage.bucket.Bucket, metadata_file_path: Path) -> Tuple[bool, str]: - local_icon_path = metadata_file_path.parent / ICON_FILE_NAME +def _icon_upload(metadata: ConnectorMetadataDefinitionV0, bucket: storage.bucket.Bucket, icon_file_path: Path) -> Tuple[bool, str]: latest_icon_path = get_icon_remote_file_path(metadata.data.dockerRepository, "latest") - if not local_icon_path.exists(): - return False, f"No Icon found at {local_icon_path}" - return upload_file_if_changed(local_icon_path, bucket, latest_icon_path) + if not icon_file_path.exists(): + return False, f"No Icon found at {icon_file_path}" + return upload_file_if_changed(icon_file_path, bucket, latest_icon_path) def _doc_upload( @@ -282,7 +281,7 @@ def upload_metadata_to_gcs(bucket_name: str, metadata_file_path: Path, validator Returns: Tuple[bool, str]: Whether the metadata file was uploaded and its blob id. """ - + icon_file_path = metadata_file_path.parent / ICON_FILE_NAME metadata_file_path = _apply_modifications_to_metadata_file(metadata_file_path, validator_opts) metadata, error = validate_and_load(metadata_file_path, POST_UPLOAD_VALIDATORS, validator_opts) @@ -299,7 +298,7 @@ def upload_metadata_to_gcs(bucket_name: str, metadata_file_path: Path, validator bucket = storage_client.bucket(bucket_name) docs_path = Path(validator_opts.docs_path) - icon_uploaded, icon_blob_id = _icon_upload(metadata, bucket, metadata_file_path) + icon_uploaded, icon_blob_id = _icon_upload(metadata, bucket, icon_file_path) version_uploaded, version_blob_id = _version_upload(metadata, bucket, metadata_file_path) From 8b4d81a43ee41855db895108daed48ad137a2c82 Mon Sep 17 00:00:00 2001 From: erohmensing Date: Wed, 15 May 2024 18:46:58 -0500 Subject: [PATCH 2/6] fix arg --- .../metadata_service/lib/metadata_service/commands.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/airbyte-ci/connectors/metadata_service/lib/metadata_service/commands.py b/airbyte-ci/connectors/metadata_service/lib/metadata_service/commands.py index 2652debfd17e2..c780a022362aa 100644 --- a/airbyte-ci/connectors/metadata_service/lib/metadata_service/commands.py +++ b/airbyte-ci/connectors/metadata_service/lib/metadata_service/commands.py @@ -15,12 +15,12 @@ def log_metadata_upload_info(metadata_upload_info: MetadataUploadInfo): for file in metadata_upload_info.uploaded_files: if file.uploaded: click.secho( - f"The {file.description} file for {metadata_upload_info.metadata_file_path} was uploaded to {file.blob_id}.", color="green" + f"The {file.description} file for {metadata_upload_info.metadata_file_path} was uploaded to {file.blob_id}.", fg="green" ) else: click.secho( f"The {file.description} file for {metadata_upload_info.metadata_file_path} was not uploaded. Reason: {file.blob_id}", - color="yellow", + fg="yellow", ) From be66d56b7837d60cb9177d1737486c93aea21fd1 Mon Sep 17 00:00:00 2001 From: erohmensing Date: Thu, 16 May 2024 09:46:21 -0500 Subject: [PATCH 3/6] bump toml version --- airbyte-ci/connectors/metadata_service/lib/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airbyte-ci/connectors/metadata_service/lib/pyproject.toml b/airbyte-ci/connectors/metadata_service/lib/pyproject.toml index a7123d3af472d..ab9d09e59f019 100644 --- a/airbyte-ci/connectors/metadata_service/lib/pyproject.toml +++ b/airbyte-ci/connectors/metadata_service/lib/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "metadata-service" -version = "0.7.0" +version = "0.7.1" description = "" authors = ["Ben Church "] readme = "README.md" From cbe46539761f97a564222f95b6f16c8afd28042a Mon Sep 17 00:00:00 2001 From: erohmensing Date: Thu, 16 May 2024 11:44:05 -0500 Subject: [PATCH 4/6] update tests do reflect that we now log when we don't upload things --- .../lib/metadata_service/commands.py | 3 +- .../lib/tests/test_commands.py | 61 +++++++++++++------ 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/airbyte-ci/connectors/metadata_service/lib/metadata_service/commands.py b/airbyte-ci/connectors/metadata_service/lib/metadata_service/commands.py index c780a022362aa..d17cb489075e5 100644 --- a/airbyte-ci/connectors/metadata_service/lib/metadata_service/commands.py +++ b/airbyte-ci/connectors/metadata_service/lib/metadata_service/commands.py @@ -58,10 +58,9 @@ def upload(metadata_file_path: pathlib.Path, docs_path: pathlib.Path, bucket_nam upload_info = upload_metadata_to_gcs(bucket_name, metadata_file_path, validator_opts) log_metadata_upload_info(upload_info) except (ValidationError, FileNotFoundError) as e: - click.secho(f"The metadata file could not be uploaded: {str(e)}", color="red") + click.secho(f"The metadata file could not be uploaded: {str(e)}", fg="red") exit(1) if upload_info.metadata_uploaded: exit(0) else: - click.secho(f"The metadata file {metadata_file_path} was not uploaded.", color="yellow") exit(5) diff --git a/airbyte-ci/connectors/metadata_service/lib/tests/test_commands.py b/airbyte-ci/connectors/metadata_service/lib/tests/test_commands.py index 2ef3ec4188fad..3831a21b82276 100644 --- a/airbyte-ci/connectors/metadata_service/lib/tests/test_commands.py +++ b/airbyte-ci/connectors/metadata_service/lib/tests/test_commands.py @@ -26,16 +26,16 @@ def test_valid_metadata_yaml_files(mocker, valid_metadata_yaml_files, tmp_path): result = runner.invoke(commands.validate, [file_path, str(tmp_path)]) assert result.exit_code == 0, f"Validation failed for {file_path} with error: {result.output}" - -def test_invalid_metadata_yaml_files(invalid_metadata_yaml_files, tmp_path): - runner = CliRunner() - - assert len(invalid_metadata_yaml_files) > 0, "No files found" - - 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 should have failed) for {file_path}" - +# +# def test_invalid_metadata_yaml_files(invalid_metadata_yaml_files, tmp_path): +# runner = CliRunner() +# +# assert len(invalid_metadata_yaml_files) > 0, "No files found" +# +# 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 should have failed) for {file_path}" +# def test_metadata_file_not_found_fails(tmp_path): runner = CliRunner() @@ -160,47 +160,70 @@ def test_upload( if latest_uploaded: commands.click.secho.assert_has_calls( - [mocker.call(f"The latest metadata file for {metadata_file_path} was uploaded to latest_blob_id.", color="green")] + [mocker.call(f"The latest metadata file for {metadata_file_path} was uploaded to latest_blob_id.", fg="green")] ) assert result.exit_code == 0 + else: + commands.click.secho.assert_has_calls( + [mocker.call(f"The latest metadata file for {metadata_file_path} was not uploaded. Reason: None", fg="yellow")] + ) if version_uploaded: commands.click.secho.assert_has_calls( - [mocker.call(f"The versioned metadata file for {metadata_file_path} was uploaded to version_blob_id.", color="green")] + [mocker.call(f"The versioned metadata file for {metadata_file_path} was uploaded to version_blob_id.", fg="green")] ) assert result.exit_code == 0 + else: + commands.click.secho.assert_has_calls( + [mocker.call(f"The versioned metadata file for {metadata_file_path} was not uploaded. Reason: None", fg="yellow")] + ) if icon_uploaded: commands.click.secho.assert_has_calls( - [mocker.call(f"The icon file for {metadata_file_path} was uploaded to icon_blob_id.", color="green")] + [mocker.call(f"The icon file for {metadata_file_path} was uploaded to icon_blob_id.", fg="green")] ) + else: + commands.click.secho.assert_has_calls([mocker.call(f"The icon file for {metadata_file_path} was not uploaded. Reason: None", fg="yellow")]) if doc_version_uploaded: commands.click.secho.assert_has_calls( - [mocker.call(f"The versioned doc file for {metadata_file_path} was uploaded to doc_version_blob_id.", color="green")] + [mocker.call(f"The versioned doc file for {metadata_file_path} was uploaded to doc_version_blob_id.", fg="green")] + ) + else: + commands.click.secho.assert_has_calls( + [mocker.call(f"The versioned doc file for {metadata_file_path} was not uploaded. Reason: None", fg="yellow")] ) if doc_inapp_version_uploaded: commands.click.secho.assert_has_calls( [ mocker.call( - f"The versioned inapp doc file for {metadata_file_path} was uploaded to doc_inapp_version_blob_id.", color="green" + f"The versioned inapp doc file for {metadata_file_path} was uploaded to doc_inapp_version_blob_id.", fg="green" ) ] ) + else: + commands.click.secho.assert_has_calls( + [mocker.call(f"The versioned inapp doc file for {metadata_file_path} was not uploaded. Reason: None", fg="yellow")] + ) if doc_latest_uploaded: commands.click.secho.assert_has_calls( - [mocker.call(f"The latest doc file for {metadata_file_path} was uploaded to doc_latest_blob_id.", color="green")] + [mocker.call(f"The latest doc file for {metadata_file_path} was uploaded to doc_latest_blob_id.", fg="green")] ) + else: + commands.click.secho.assert_has_calls([mocker.call(f"The latest doc file for {metadata_file_path} was not uploaded. Reason: None", fg="yellow")]) if doc_inapp_latest_uploaded: commands.click.secho.assert_has_calls( - [mocker.call(f"The latest inapp doc file for {metadata_file_path} was uploaded to doc_inapp_latest_blob_id.", color="green")] + [mocker.call(f"The latest inapp doc file for {metadata_file_path} was uploaded to doc_inapp_latest_blob_id.", fg="green")] + ) + else: + commands.click.secho.assert_has_calls( + [mocker.call(f"The latest inapp doc file for {metadata_file_path} was not uploaded. Reason: None", fg="yellow")] ) if not (latest_uploaded or version_uploaded): - commands.click.secho.assert_has_calls([mocker.call(f"The metadata file {metadata_file_path} was not uploaded.", color="yellow")]) # We exit with 5 status code to share with the CI pipeline that the upload was skipped. assert result.exit_code == 5 @@ -243,4 +266,4 @@ def test_upload_with_errors(mocker, valid_metadata_yaml_files, tmp_path, error, ) # Using valid_metadata_yaml_files[0] as SA because it exists... assert result.exit_code == 1 if handled: - commands.click.secho.assert_called_with(f"The metadata file could not be uploaded: {str(error)}", color="red") + commands.click.secho.assert_called_with(f"The metadata file could not be uploaded: {str(error)}", fg="red") From bdf3ac0432df50195d3d48c46525792b1c26ca8f Mon Sep 17 00:00:00 2001 From: erohmensing Date: Thu, 16 May 2024 11:45:31 -0500 Subject: [PATCH 5/6] format --- .../metadata_service/lib/tests/test_commands.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/airbyte-ci/connectors/metadata_service/lib/tests/test_commands.py b/airbyte-ci/connectors/metadata_service/lib/tests/test_commands.py index 3831a21b82276..bd6d5f6e8b56e 100644 --- a/airbyte-ci/connectors/metadata_service/lib/tests/test_commands.py +++ b/airbyte-ci/connectors/metadata_service/lib/tests/test_commands.py @@ -26,6 +26,7 @@ def test_valid_metadata_yaml_files(mocker, valid_metadata_yaml_files, tmp_path): result = runner.invoke(commands.validate, [file_path, str(tmp_path)]) assert result.exit_code == 0, f"Validation failed for {file_path} with error: {result.output}" + # # def test_invalid_metadata_yaml_files(invalid_metadata_yaml_files, tmp_path): # runner = CliRunner() @@ -37,6 +38,7 @@ def test_valid_metadata_yaml_files(mocker, valid_metadata_yaml_files, tmp_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)]) @@ -183,7 +185,9 @@ def test_upload( [mocker.call(f"The icon file for {metadata_file_path} was uploaded to icon_blob_id.", fg="green")] ) else: - commands.click.secho.assert_has_calls([mocker.call(f"The icon file for {metadata_file_path} was not uploaded. Reason: None", fg="yellow")]) + commands.click.secho.assert_has_calls( + [mocker.call(f"The icon file for {metadata_file_path} was not uploaded. Reason: None", fg="yellow")] + ) if doc_version_uploaded: commands.click.secho.assert_has_calls( @@ -196,11 +200,7 @@ def test_upload( if doc_inapp_version_uploaded: commands.click.secho.assert_has_calls( - [ - mocker.call( - f"The versioned inapp doc file for {metadata_file_path} was uploaded to doc_inapp_version_blob_id.", fg="green" - ) - ] + [mocker.call(f"The versioned inapp doc file for {metadata_file_path} was uploaded to doc_inapp_version_blob_id.", fg="green")] ) else: commands.click.secho.assert_has_calls( @@ -212,7 +212,9 @@ def test_upload( [mocker.call(f"The latest doc file for {metadata_file_path} was uploaded to doc_latest_blob_id.", fg="green")] ) else: - commands.click.secho.assert_has_calls([mocker.call(f"The latest doc file for {metadata_file_path} was not uploaded. Reason: None", fg="yellow")]) + commands.click.secho.assert_has_calls( + [mocker.call(f"The latest doc file for {metadata_file_path} was not uploaded. Reason: None", fg="yellow")] + ) if doc_inapp_latest_uploaded: commands.click.secho.assert_has_calls( From b1f54f2325f4b985f68fd51106392ca08364e053 Mon Sep 17 00:00:00 2001 From: erohmensing Date: Thu, 16 May 2024 11:47:37 -0500 Subject: [PATCH 6/6] un-comment slow tests --- .../lib/tests/test_commands.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/airbyte-ci/connectors/metadata_service/lib/tests/test_commands.py b/airbyte-ci/connectors/metadata_service/lib/tests/test_commands.py index bd6d5f6e8b56e..1cdafe1612d39 100644 --- a/airbyte-ci/connectors/metadata_service/lib/tests/test_commands.py +++ b/airbyte-ci/connectors/metadata_service/lib/tests/test_commands.py @@ -27,16 +27,14 @@ def test_valid_metadata_yaml_files(mocker, valid_metadata_yaml_files, tmp_path): assert result.exit_code == 0, f"Validation failed for {file_path} with error: {result.output}" -# -# def test_invalid_metadata_yaml_files(invalid_metadata_yaml_files, tmp_path): -# runner = CliRunner() -# -# assert len(invalid_metadata_yaml_files) > 0, "No files found" -# -# 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 should have failed) for {file_path}" -# +def test_invalid_metadata_yaml_files(invalid_metadata_yaml_files, tmp_path): + runner = CliRunner() + + assert len(invalid_metadata_yaml_files) > 0, "No files found" + + 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 should have failed) for {file_path}" def test_metadata_file_not_found_fails(tmp_path):