kaxil commented on code in PR #76020:
URL: https://github.com/apache/airflow/pull/76020#discussion_r4437465026
##########
dev/breeze/src/airflow_breeze/global_constants.py:
##########
@@ -362,6 +362,12 @@ class TarBallType(Enum):
PACKAGES_METADATA_EXCLUDE_NAMES = ["docker-stack", "apache-airflow-providers"]
+# Published docs folders that are not PEP 440 versions, such as Maven
snapshots staged for testing.
+# Any other invalid version folder still fails the docs publishing.
+PACKAGES_METADATA_EXCLUDE_VERSIONS: dict[str, list[str]] = {
+ "java-sdk": ["1.0.0-SNAPSHOT"],
Review Comment:
This folder name comes from `projectVersion` in
`java-sdk/gradle.properties`, and `java-sdk/DEVELOPMENT.md` says `main` stays
on a `-SNAPSHOT` between releases. So after the first release, `main` moves to
the next snapshot, staging gets a new snapshot folder that isn't listed here,
and the job exits 1 again (the second test asserts exactly that). Would
matching the suffix per package work instead, e.g. `{"java-sdk":
("-SNAPSHOT",)}` checked with `v.endswith(...)`? Any other invalid folder would
still fail the job.
##########
dev/breeze/tests/test_publish_docs_to_s3.py:
##########
@@ -298,3 +298,34 @@ def test_run_publish_packages_metadata(
self.publish_docs_to_s3.run_publish()
assert mock_generate_packages_metadata.call_count == expected_calls
+
+ @patch.dict(
+
"airflow_breeze.utils.publish_docs_to_s3.PACKAGES_METADATA_EXCLUDE_VERSIONS",
+ {"java-sdk": ["1.0.0-SNAPSHOT"], "snapshot-only-sdk":
["1.0.0-SNAPSHOT"]},
+ clear=True,
+ )
+ @patch.object(VersionError, "version_error", False)
+ def test_dump_docs_package_metadata_skips_excluded_versions(self):
+ metadata = self.publish_docs_to_s3.dump_docs_package_metadata(
+ {
+ "java-sdk": ["1.0.0-SNAPSHOT", "1.0.0-beta1"],
+ "snapshot-only-sdk": ["1.0.0-SNAPSHOT"],
+ }
+ )
+
+ assert metadata == [
+ {"package-name": "java-sdk", "all-versions": ["1.0.0b1"],
"stable-version": "1.0.0b1"},
+ ]
+ assert not VersionError.has_any_error()
+
+ @patch.dict(
+
"airflow_breeze.utils.publish_docs_to_s3.PACKAGES_METADATA_EXCLUDE_VERSIONS",
+ {"java-sdk": ["1.0.0-SNAPSHOT"]},
+ clear=True,
+ )
+ @patch.object(VersionError, "version_error", False)
+ def test_get_latest_minor_versions_flags_versions_not_excluded(self):
Review Comment:
Nothing here covers a version that's allowlisted for one package showing up
under a different one. If `.get(package_name, [])` became a flat set of every
allowlisted version, both tests would still pass. Could you add a case with the
same snapshot folder under a package that isn't in the map and assert
`VersionError.has_any_error()`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
