connectors-ci: new checks to validate connector version #25609
Conversation
|
|
||
| @cached_property | ||
| def master_metadata(self) -> dict: | ||
| response = requests.get(self.github_master_metadata_url) |
There was a problem hiding this comment.
@bnchrch I considered reading the metadata from the GCS bucket using the latest key. I decided to use GitHub as this check would not rely on the orchestrator job runs, but let me know if you think otherwise.
There was a problem hiding this comment.
I think this was the right call :)
| for not_committed_change in status_output.split("\n"): | ||
| file_path = not_committed_change.strip().split(" ")[-1] | ||
| if file_path: | ||
| modified_files.append(file_path) |
There was a problem hiding this comment.
This new change is to not force developers to commit their code to detect locally modified files.
| @@ -16,6 +16,7 @@ This documentation should be helpful for both local and CI use of the CLI. We in | |||
| ### Install | |||
| ```bash | |||
| # Make sure that the current Python version is >= 3.10 | |||
| pyenv shell 3.10 | |||
There was a problem hiding this comment.
Unrelated doc addition 😄
This reverts commit 0950d39.
| @@ -108,6 +109,11 @@ Test connectors changed on the current branch: | |||
| ```mermaid | |||
| flowchart TD | |||
| entrypoint[[For each selected connector]] | |||
| subgraph version ["Connector version checks"] | |||
| List[StepResult]: The results of the version checks steps. | ||
| """ | ||
| context.logger.info("Run version checks.") | ||
| return [await VersionFollowsSemverCheck(context).run(), await VersionIncrementCheck(context).run()] |
| @@ -87,13 +100,18 @@ async def run_connector_test_pipeline(context: ConnectorContext, semaphore: anyi | |||
| """ | |||
| async with semaphore: | |||
| async with context: | |||
| version_checks_results = await run_version_checks(context) | |||
There was a problem hiding this comment.
Just thinking about the skip on failure login in run_tests, could we use that here?
There was a problem hiding this comment.
I agree but in in this context I think run_steps is not immediately compatible as I wrapped steps executions inside function like run_versions_checks, run_qa_checks etc.
I think we might eventually want to make these function Steps themselves (or Pipeline). And a Pipeline could nest multiple other pipelines execution.
|
|
||
| @cached_property | ||
| def master_metadata(self) -> dict: | ||
| response = requests.get(self.github_master_metadata_url) |
There was a problem hiding this comment.
I think this was the right call :)
tools/ci_connector_ops/ci_connector_ops/pipelines/tests/common.py
Outdated
Show resolved
Hide resolved
|
@bnchrch I did the following changes:
I also realize that we'll probably soon want to migrate the QA checks into specific pipeline steps because the pipelines have awareness on the modified files and it'll help add smarter static code analysis checks. Finally, I opened #25661 to encourage connector developers to bump the version in the metadata before we discard /test. |
) * connectors-ci: new checks to validate connector version * tmp ref attempt wip * Revert "tmp ref attempt wip" This reverts commit 0950d39. * use semver and bypass increment check for specific files + no early fail
What
Closes #25329
When a connector is modified we want to enforce a connector version increment in the
metadata.yamlfile in thedockerImageTagfield.The version that will eventually be published on merge is pulled from the metadata file, it's why we want to check the version is bumped in this file.
How
If the CI context is not master and connectors files were modified:
dockerImageTagin themetadata.yamlfile follows semantic versionningmasterversion ofmetadata.yamlfromraw.github.com