Merge branch 'master' into geospatial-aggregate-output

# Conflicts: # CHANGELOG.md # cognite/client/_version.py # pyproject.toml
cognitedata · Sep 26, 2022 · 88b6a92 · 88b6a92
2 parents 9cd0ce3 + 9f1316e
commit 88b6a92
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 74 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,10 +14,19 @@ Changes are grouped as follows
 - `Fixed` for any bug fixes.
 - `Security` in case of vulnerabilities.
 
-## [4.6.0] - 2022-09-15
+## [4.6.0] - 2022-09-26
 ### Changed
 - Change geospatial.aggregate_features to support `aggregate_output`
 
+## [4.5.4] - 2022-09-19
+### Fixed
+- The raw rows insert endpoint is now subject to the same retry logic as other idempotent endpoints.
+
+## [4.5.3] - 2022-09-15
+### Fixed
+- Fixes the OS specific issue where the `requirements.txt`-validation failed 
+  with `Permission Denied` on Windows.
+
 ## [4.5.2] - 2022-09-09
 ### Fixed
 - Fixes the issue when updating transformations with new nonce credentials

diff --git a/cognite/client/_api/assets.py b/cognite/client/_api/assets.py
@@ -395,7 +395,7 @@ def delete(
 
         Args:
             id (Union[int, Sequence[int]): Id or list of ids
-            external_id (Union[str, Sequence[str]]): External ID or list of exgernal ids
+            external_id (Union[str, Sequence[str]]): External ID or list of external ids
             recursive (bool): Recursively delete whole asset subtrees under given ids. Defaults to False.
             ignore_unknown_ids (bool): Ignore IDs and external IDs that are not found rather than throw an exception.
 

diff --git a/cognite/client/_api/functions.py b/cognite/client/_api/functions.py
@@ -7,7 +7,7 @@
 from numbers import Integral, Number
 from pathlib import Path
 from tempfile import NamedTemporaryFile, TemporaryDirectory
-from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Sequence, Union, cast
+from typing import IO, TYPE_CHECKING, Any, Callable, Dict, List, Optional, Sequence, Union, cast
 from zipfile import ZipFile
 
 from cognite.client import utils
@@ -443,8 +443,8 @@ def _zip_and_upload_folder(self, folder: str, name: str, external_id: Optional[s
                             reqs = _extract_requirements_from_file(path)
                             # Validate and format requirements
                             parsed_reqs = _validate_and_parse_requirements(reqs)
-                            with NamedTemporaryFile() as nth:
-                                _write_requirements_to_file(nth.name, parsed_reqs)
+                            with NamedTemporaryFile(mode="w+") as nth:
+                                _write_requirements_to_named_temp_file(nth, parsed_reqs)
                                 # NOTE: the actual file is not written.
                                 # A temporary formatted file is used instead
                                 zf.write(nth.name, arcname=REQUIREMENTS_FILE_NAME)
@@ -477,8 +477,8 @@ def _zip_and_upload_handle(self, function_handle: Callable, name: str, external_
                 f.write(source)
 
             # Read and validate requirements
-            with NamedTemporaryFile() as named_temp_file:
-                requirements_written = _write_fn_docstring_requirements_to_file(function_handle, named_temp_file.name)
+            with NamedTemporaryFile(mode="w+") as named_temp_file:
+                requirements_written = _write_fn_docstring_requirements_to_file(function_handle, named_temp_file)
 
                 zip_path = os.path.join(tmpdir, "function.zip")
                 with ZipFile(zip_path, "w") as zf:
@@ -729,12 +729,11 @@ def _validate_and_parse_requirements(requirements: List[str]) -> List[str]:
     return parsed_reqs
 
 
-def _write_requirements_to_file(file_path: str, requirements: List[str]) -> None:
-    with open(file_path, "w+") as f:
-        f.write("\n".join(requirements))
+def _write_requirements_to_named_temp_file(file: IO, requirements: List[str]) -> None:
+    file.write("\n".join(requirements))
 
 
-def _write_fn_docstring_requirements_to_file(fn: Callable, file_path: str) -> bool:
+def _write_fn_docstring_requirements_to_file(fn: Callable, file: IO) -> bool:
     """Read requirements from a function docstring, validate them, and write contents to the provided file path
 
     Args:
@@ -750,7 +749,7 @@ def _write_fn_docstring_requirements_to_file(fn: Callable, file_path: str) -> bo
         reqs = _extract_requirements_from_doc_string(docstr)
         if reqs:
             parsed_reqs = _validate_and_parse_requirements(reqs)
-            _write_requirements_to_file(file_path, parsed_reqs)
+            _write_requirements_to_named_temp_file(file, parsed_reqs)
             return True
 
     return False

diff --git a/cognite/client/_api/transformations/schedules.py b/cognite/client/_api/transformations/schedules.py
@@ -150,7 +150,7 @@ def delete(
 
         Args:
             id (Union[int, Sequence[int]): Id or list of ids
-            external_id (Union[str, Sequence[str]]): External ID or list of exgernal ids
+            external_id (Union[str, Sequence[str]]): External ID or list of external ids
             ignore_unknown_ids (bool): Ignore IDs and external IDs that are not found rather than throw an exception.
 
         Returns:

diff --git a/cognite/client/_api_client.py b/cognite/client/_api_client.py
@@ -1,3 +1,4 @@
+import functools
 import gzip
 import json as _json
 import logging
@@ -54,38 +55,19 @@ class APIClient:
     _RESOURCE_PATH: str
 
     # TODO: This following set should be generated from the openapi spec somehow.
-    RETRYABLE_POST_ENDPOINTS = {
-        "/assets/list",
-        "/assets/byids",
-        "/assets/search",
-        "/events/list",
-        "/events/byids",
-        "/events/search",
-        "/files/list",
-        "/files/byids",
-        "/files/search",
-        "/files/downloadlink",
-        "/timeseries/byids",
-        "/timeseries/search",
-        "/timeseries/list",
-        "/timeseries/data",
-        "/timeseries/data/list",
-        "/timeseries/data/latest",
-        "/timeseries/data/delete",
-        "/sequences/byids",
-        "/sequences/search",
-        "/sequences/data",
-        "/sequences/data/list",
-        "/sequences/data/delete",
-        "/datasets/list",
-        "/datasets/aggregate",
-        "/datasets/byids",
-        "/relationships/list",
-        "/relationships/byids",
-        "/context/entitymatching/byids",
-        "/context/entitymatching/list",
-        "/context/entitymatching/jobs",
-        "/sessions/revoke",
+    _RETRYABLE_POST_ENDPOINT_REGEX_PATTERNS = {
+        "^" + path + "(\?.*)?$"
+        for path in (
+            "/(assets|events|files|timeseries|sequences|datasets|relationships)/(list|byids|search|aggregate)",
+            "/files/downloadlink",
+            "/timeseries/data",
+            "/timeseries/data/(list|latest|delete)",
+            "/sequences/data",
+            "/sequences/data/(list|delete)",
+            "/raw/dbs/[^/]+/tables/[^/]+",
+            "/context/entitymatching/(byids|list|jobs)",
+            "/sessions/revoke",
+        )
     }
 
     def __init__(
@@ -230,21 +212,31 @@ def _get_base_url_with_base_path(self) -> str:
 
     def _is_retryable(self, method: str, path: str) -> bool:
         valid_methods = ["GET", "POST", "PUT", "DELETE", "PATCH"]
-        match = re.match(
-            r"(?:http|https)://[a-z\d.:\-]+(?:/api/(?:v1|playground)/projects/[^/]+)?(/[^\?]+)?(\?.+)?", path
-        )
-        if not match:
-            raise ValueError("Path {} is not valid. Cannot resolve whether or not it is retryable".format(path))
+
         if method not in valid_methods:
             raise ValueError("Method {} is not valid. Must be one of {}".format(method, valid_methods))
-        path_end = match.group(1)
 
         if method in ["GET", "PUT", "PATCH"]:
             return True
-        if method == "POST" and path_end in self.RETRYABLE_POST_ENDPOINTS:
+        if method == "POST" and self._url_is_retryable(path):
             return True
         return False
 
+    @classmethod
+    @functools.lru_cache(64)
+    def _url_is_retryable(cls, url: str) -> bool:
+        valid_url_pattern = (
+            r"^(?:http|https)://[a-z\d.:\-]+(?:/api/(?:v1|playground)/projects/[^/]+)?((/[^\?]+)?(\?.+)?)"
+        )
+        match = re.match(valid_url_pattern, url)
+        if not match:
+            raise ValueError("URL {} is not valid. Cannot resolve whether or not it is retryable".format(url))
+        path = match.group(1)
+        for pattern in cls._RETRYABLE_POST_ENDPOINT_REGEX_PATTERNS:
+            if re.match(pattern, path):
+                return True
+        return False
+
     def _retrieve(
         self,
         identifier: Identifier,

diff --git a/tests/tests_unit/test_api/test_functions.py b/tests/tests_unit/test_api/test_functions.py
@@ -688,15 +688,15 @@ def fn():
             """
             return None
 
-        with NamedTemporaryFile() as ntf:
-            assert _write_fn_docstring_requirements_to_file(fn, ntf.name) is True
+        with NamedTemporaryFile(mode="w+") as ntf:
+            assert _write_fn_docstring_requirements_to_file(fn, ntf) is True
 
     def test_get_requirements_handle_error(self):
         def fn():
             return None
 
-        with NamedTemporaryFile() as ntf:
-            assert _write_fn_docstring_requirements_to_file(fn, ntf.name) is False
+        with NamedTemporaryFile(mode="w+") as ntf:
+            assert _write_fn_docstring_requirements_to_file(fn, ntf) is False
 
     def test_get_requirements_handle_no_docstr(self):
         def fn():
@@ -708,8 +708,8 @@ def fn():
             return None
 
         with pytest.raises(Exception):
-            with NamedTemporaryFile() as ntf:
-                assert _write_fn_docstring_requirements_to_file(fn, ntf.name) is False
+            with NamedTemporaryFile(mode="w+") as ntf:
+                assert _write_fn_docstring_requirements_to_file(fn, ntf) is False
 
     def test_get_requirements_handle_no_reqs(self):
         def fn():
@@ -719,8 +719,8 @@ def fn():
             """
             return None
 
-        with NamedTemporaryFile() as ntf:
-            assert _write_fn_docstring_requirements_to_file(fn, ntf.name) is False
+        with NamedTemporaryFile(mode="w+") as ntf:
+            assert _write_fn_docstring_requirements_to_file(fn, ntf) is False
 
     def test_extract_requirements_from_file(self, tmpdir):
         req = "somepackage == 3.8.1"

diff --git a/tests/tests_unit/test_api_client.py b/tests/tests_unit/test_api_client.py
@@ -1147,6 +1147,14 @@ class TestHelpers:
             ("PUT", "https://localhost:8000.com/api/v1/projects/blabla/assets", True),
             ("PATCH", "https://localhost:8000.com/api/v1/projects/blabla/patchy", True),
             ("GET", "https://another-cluster.cognitedata.com/login/status", True),
+            ("POST", "https://api.cognitedata.com/api/v1/projects/bla/raw/dbs/mydb/tables/mytable", True),
+            ("POST", "https://api.cognitedata.com/api/v1/projects/bla/assets/list", True),
+            ("POST", "https://api.cognitedata.com/api/v1/projects/bla/events/byids", True),
+            ("POST", "https://api.cognitedata.com/api/v1/projects/bla/files/search", True),
+            ("POST", "https://api.cognitedata.com/api/v1/projects/bla/timeseries/list", True),
+            ("POST", "https://api.cognitedata.com/api/v1/projects/bla/sequences/byids", True),
+            ("POST", "https://api.cognitedata.com/api/v1/projects/bla/datasets/aggregate", True),
+            ("POST", "https://api.cognitedata.com/api/v1/projects/bla/relationships/list", True),
         ],
     )
     def test_is_retryable(self, api_client_with_api_key, method, path, expected):
@@ -1160,26 +1168,13 @@ def test_is_retryable_fail(self, api_client_with_api_key, method, path):
             api_client_with_api_key._is_retryable(method, path)
 
     def test_is_retryable_add(self, api_client_with_api_key):
-        assert (
-            api_client_with_api_key._is_retryable(
-                "POST", "https://greenfield.cognitedata.com/api/v1/projects/blabla/assets/bloop"
-            )
-            is False
-        )
-        APIClient.RETRYABLE_POST_ENDPOINTS.add("/assets/bloop")
+        APIClient._RETRYABLE_POST_ENDPOINT_REGEX_PATTERNS.add("/assets/bloop")
         assert (
             api_client_with_api_key._is_retryable(
                 "POST", "https://greenfield.cognitedata.com/api/v1/projects/blabla/assets/bloop"
             )
             is True
         )
-        APIClient.RETRYABLE_POST_ENDPOINTS.remove("/assets/bloop")
-        assert (
-            api_client_with_api_key._is_retryable(
-                "POST", "https://greenfield.cognitedata.com/api/v1/projects/blabla/assets/bloop"
-            )
-            is False
-        )
 
     @pytest.mark.parametrize(
         "before, after",