weiqingy commented on code in PR #1005:
URL: https://github.com/apache/flink-agents/pull/1005#discussion_r3840665754
##########
python/flink_agents/api/tests/test_skills.py:
##########
@@ -30,12 +32,53 @@ def test_from_local_dir_emits_local_scheme(self) -> None:
def test_from_url_emits_url_scheme(self) -> None:
s = Skills.from_url("https://example.com/x.zip";)
+ assert s.sources == [
+ SkillSourceSpec(scheme="url", params={"url":
"https://example.com/x.zip"})
+ ]
+
+ def test_from_url_with_sha256_emits_integrity_param(self) -> None:
+ digest = "A" * 64
+ s = Skills.from_url_with_sha256("https://example.com/x.zip";, digest)
assert s.sources == [
SkillSourceSpec(
- scheme="url", params={"url": "https://example.com/x.zip"}
+ scheme="url",
+ params={"url": "https://example.com/x.zip";, "sha256": digest},
)
]
+ def test_from_url_unsafe_requires_explicit_param(self) -> None:
+ s = Skills.from_url_unsafe("http://example.com/x.zip";)
+ assert s.sources[0].params["allow_insecure_http"] == "true"
+
+ def test_from_url_unsafe_with_sha256_emits_both_params(self) -> None:
+ digest = "a" * 64
+ s = Skills.from_url_unsafe_with_sha256("http://example.com/x.zip";,
digest)
+ assert s.sources[0].params == {
+ "url": "http://example.com/x.zip";,
+ "sha256": digest,
+ "allow_insecure_http": "true",
+ }
+
+ def test_from_url_rejects_plain_http_by_default(self) -> None:
+ with pytest.raises(ValueError, match="disabled by default"):
+ Skills.from_url("http://example.com/x.zip";)
+
+ def test_from_url_with_sha256_rejects_malformed_digest(self) -> None:
+ with pytest.raises(ValueError, match="64 hexadecimal"):
+ Skills.from_url_with_sha256("https://example.com/x.zip";, "invalid")
+
+ def test_from_url_rejects_unsupported_scheme_clearly(self) -> None:
+ with pytest.raises(ValueError, match=r"Only HTTP\(S\)"):
+ Skills.from_url("ftp://example.com/x.zip";)
+
+ @pytest.mark.parametrize(
+ "url",
+ ["https://exa mple.com/x.zip", "https://example.com/%invalid";],
Review Comment:
nit: both of these cases hit the two new regexes at `skills.py:189`, so the
`except ValueError` just above at `skills.py:186-188` still has nothing
exercising it. That branch is the only thing handling a URL `urlparse` itself
refuses: `urlparse("https://[::1/x.zip";)` raises `Invalid IPv6 URL` and neither
regex matches that string, while `URI.create` rejects the same string as
`Expected closing bracket for IPv6 address`.
Adding it to this list would cover it. Nothing asserts `Invalid skill URL:`
on the Java side either (`Skills.java:165-167`), if you want one per language
like last time.
--
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]
