[GitHub] [airflow] ashb commented on a diff in pull request #29497: Check that cloud sql provider version is valid

["ashb (via GitHub)" <gi...@apache.org>]

ashb commented on code in PR #29497:
URL: https://github.com/apache/airflow/pull/29497#discussion_r1104045223


##########
airflow/providers/google/cloud/hooks/cloud_sql.py:
##########
@@ -482,6 +475,24 @@ def _download_sql_proxy_if_needed(self) -> None:
         os.chmod(self.sql_proxy_path, 0o744)  # Set executable bit
         self.sql_proxy_was_downloaded = True
 
+    def _get_sql_proxy_download_url(self):
+        system = platform.system().lower()
+        processor = os.uname().machine
+        if processor == "x86_64":
+            processor = "amd64"
+        if not self.sql_proxy_version:
+            download_url = CLOUD_SQL_PROXY_DOWNLOAD_URL.format(system, processor)
+        else:
+            if not CLOUD_SQL_PROXY_VERSION_REGEX.match(self.sql_proxy_version):
+                raise Exception(

Review Comment:
   Nit:
   
   ```suggestion
                   raise ValueError(
   ```
   



##########
tests/providers/google/cloud/hooks/test_cloud_sql.py:
##########
@@ -1185,3 +1200,53 @@ def test_hook_with_correct_parameters_mysql_tcp(self, get_connection):
         assert "127.0.0.1" == connection.host
         assert 3200 != connection.port
         assert "testdb" == connection.schema
+
+
+def get_processor():
+    processor = os.uname().machine
+    if processor == "x86_64":
+        processor = "amd64"
+    return processor
+
+
+class TestCloudSqlProxyRunner:
+    @pytest.mark.parametrize(
+        "version,download_url",
+        [
+            (
+                "v1.23.0",
+                "https://storage.googleapis.com/cloudsql-proxy/v1.23.0/cloud_sql_proxy."
+                f"{platform.system().lower()}.{get_processor()}",
+            ),
+            (
+                "v1.23.0-preview.1",
+                "https://storage.googleapis.com/cloudsql-proxy/v1.23.0-preview.1/cloud_sql_proxy."
+                f"{platform.system().lower()}.{get_processor()}",
+            ),
+        ],
+    )
+    def test_cloud_sql_proxy_runner_version_ok(self, version, download_url):
+        runner = CloudSqlProxyRunner(
+            path_prefix="12345678",
+            instance_specification="project:us-east-1:instance",
+            sql_proxy_version=version,
+        )
+        assert runner._get_sql_proxy_download_url() == download_url
+
+    @pytest.mark.parametrize(
+        "version",
+        [
+            "v1.23.",
+            "v1.23.0..",
+            "v1.23.0\\",
+            "\\",
+        ],
+    )
+    def test_cloud_sql_proxy_runner_version_nok(self, version):
+        runner = CloudSqlProxyRunner(
+            path_prefix="12345678",
+            instance_specification="project:us-east-1:instance",
+            sql_proxy_version=version,
+        )
+        with pytest.raises(Exception, match="The sql_proxy_version should match the regular expression"):

Review Comment:
   ```suggestion
           with pytest.raises(ValueError match="The sql_proxy_version should match the regular expression"):
   ```
   



##########
tests/providers/google/cloud/hooks/test_cloud_sql.py:
##########
@@ -1185,3 +1200,53 @@ def test_hook_with_correct_parameters_mysql_tcp(self, get_connection):
         assert "127.0.0.1" == connection.host
         assert 3200 != connection.port
         assert "testdb" == connection.schema
+
+
+def get_processor():
+    processor = os.uname().machine
+    if processor == "x86_64":
+        processor = "amd64"
+    return processor
+
+
+class TestCloudSqlProxyRunner:
+    @pytest.mark.parametrize(
+        "version,download_url",

Review Comment:
   ```suggestion
           ["version", "download_url"],
   ```
   



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org