Metadata: Ensure we run upload validate on the overrode metadata file #28583
Conversation
| latest_uploaded = False | ||
|
|
||
| if prerelease: | ||
| metadata_file_path = create_prerelease_metadata_file(metadata_file_path, prerelease) |
There was a problem hiding this comment.
nice, easier to understand now - no chance of using the existing metadata file
|
|
||
| def create_prerelease_metadata_file(metadata_file_path: Path, prerelease_tag: str) -> Path: | ||
| metadata, error = validate_and_load(metadata_file_path, []) |
There was a problem hiding this comment.
👍🏻 don't validate when we load the initial file before we've overwritten it
There was a problem hiding this comment.
this will still run it through the object validator, but not perform any extra validation (like if the docker image exists). i think that's fine
|
|
||
| def _prerelease_upload(metadata: ConnectorMetadataDefinitionV0, bucket: storage.bucket.Bucket, prerelease_tag: str) -> Tuple[bool, str]: | ||
| # replace any dockerImageTag references with the actual tag | ||
| # this includes metadata.data.dockerImageTag, metadata.data.registries[].dockerImageTag |
There was a problem hiding this comment.
Not requied, just a suggestion: i wonder if we should have a constant list of these somewhere instead of encoding them here, in case we add something later that alwo has docker image tags?
or at least maybe pull out a method overwrite_image_tags
| if not prerelease: | ||
| latest_uploaded, _latest_blob_id = _latest_upload(metadata, bucket, metadata_file_path) | ||
|
|
||
| return version_uploaded or latest_uploaded, version_blob_id |
There was a problem hiding this comment.
Will we ever see latest_uploaded here? in the case that we upload it, we'll have already uploaded the version too
There was a problem hiding this comment.
I.e. for a non prerelease, if version upload failed, we want to know about it even it latest_uploaded succeeded, and the other way around. I guess we need separate return statements for the 2 cases?
There was a problem hiding this comment.
Good callout.
It was bugging me to.
Updated to have a more robust return value
bnchrch
force-pushed
the
bnchrch/metadata-fix-upload-validation
branch
from
bdb8424 to
aad0061
Compare
…airbytehq#28583) * Ensure we do an upload validate on the overrode metadata file * Change return value of upload * Format
What
Fixes an issue where if you havent published the overrode version of the connector and go to run a prerelease then the prelease will fail because it cant find the regular docker image on dockerhub