This is an automated email from the ASF dual-hosted git repository.
fokko pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git
The following commit(s) were added to refs/heads/main by this push:
new 50077af9 Deprecate `rest.authorization-url` in favor of
`oauth2-server-uri` (#962)
50077af9 is described below
commit 50077af9cf951dffc8daa60d29b5443e8dea3093
Author: Andre Luis Anastacio <[email protected]>
AuthorDate: Thu Aug 8 05:49:04 2024 -0300
Deprecate `rest.authorization-url` in favor of `oauth2-server-uri` (#962)
* Deprecate rest.authorization-url in favor of oauth2-server-uri
* Use deprecation_message instead of deprecated for property deprecation
---
mkdocs/docs/configuration.md | 26 +++++++++++++-------------
mkdocs/docs/contributing.md | 18 ++++++++++++++++++
mkdocs/docs/how-to-release.md | 10 ++++++++++
pyiceberg/catalog/__init__.py | 6 +++---
pyiceberg/catalog/rest.py | 33 +++++++++++++++++++++++++++++++--
pyiceberg/utils/deprecated.py | 33 +++++++++++++++++++++++++--------
tests/catalog/test_rest.py | 4 ++--
tests/utils/test_deprecated.py | 14 ++++++++++++++
8 files changed, 116 insertions(+), 28 deletions(-)
diff --git a/mkdocs/docs/configuration.md b/mkdocs/docs/configuration.md
index 6bfc5671..3445b5a7 100644
--- a/mkdocs/docs/configuration.md
+++ b/mkdocs/docs/configuration.md
@@ -197,19 +197,19 @@ catalog:
<!-- markdown-link-check-disable -->
-| Key | Example | Description
|
-| ---------------------- | -------------------------------- |
--------------------------------------------------------------------------------------------------
|
-| uri | https://rest-catalog/ws | URI identifying
the REST Server
|
-| ugi | t-1234:secret | Hadoop UGI for
Hive client.
|
-| credential | t-1234:secret | Credential to
use for OAuth2 credential flow when initializing the catalog
|
-| token | FEW23.DFSDF.FSDF | Bearer token
value to use for `Authorization` header
|
-| scope | openid offline corpds:ds:profile | Desired scope of
the requested security token (default : catalog)
|
-| resource | rest_catalog.iceberg.com | URI for the
target resource or service
|
-| audience | rest_catalog | Logical name of
target resource or service
|
-| rest.sigv4-enabled | true | Sign requests to
the REST Server using AWS SigV4 protocol
|
-| rest.signing-region | us-east-1 | The region to
use when SigV4 signing a request
|
-| rest.signing-name | execute-api | The service
signing name to use when SigV4 signing a request
|
-| rest.authorization-url | https://auth-service/cc | Authentication
URL to use for client credentials authentication (default: uri +
'v1/oauth/tokens') |
+| Key | Example | Description
|
+| ------------------- | -------------------------------- |
--------------------------------------------------------------------------------------------------
|
+| uri | https://rest-catalog/ws | URI identifying the
REST Server |
+| ugi | t-1234:secret | Hadoop UGI for Hive
client. |
+| credential | t-1234:secret | Credential to use
for OAuth2 credential flow when initializing the catalog
|
+| token | FEW23.DFSDF.FSDF | Bearer token value
to use for `Authorization` header
|
+| scope | openid offline corpds:ds:profile | Desired scope of
the requested security token (default : catalog)
|
+| resource | rest_catalog.iceberg.com | URI for the target
resource or service
|
+| audience | rest_catalog | Logical name of
target resource or service
|
+| rest.sigv4-enabled | true | Sign requests to
the REST Server using AWS SigV4 protocol
|
+| rest.signing-region | us-east-1 | The region to use
when SigV4 signing a request
|
+| rest.signing-name | execute-api | The service signing
name to use when SigV4 signing a request |
+| oauth2-server-uri | https://auth-service/cc | Authentication URL
to use for client credentials authentication (default: uri + 'v1/oauth/tokens')
|
<!-- markdown-link-check-enable-->
diff --git a/mkdocs/docs/contributing.md b/mkdocs/docs/contributing.md
index beacf0f3..a716f134 100644
--- a/mkdocs/docs/contributing.md
+++ b/mkdocs/docs/contributing.md
@@ -180,6 +180,24 @@ Which will warn:
Call to load_something, deprecated in 0.1.0, will be removed in 0.2.0. Please
use load_something_else() instead.
```
+If you want to remove a property or notify about a behavior change, please add
a deprecation notice by calling the deprecation_message function:
+
+```python
+from pyiceberg.utils.deprecated import deprecation_message
+
+deprecation_message(
+ deprecated_in="0.1.0",
+ removed_in="0.2.0",
+ help_message="The old_property is deprecated. Please use the
something_else property instead.",
+)
+```
+
+Which will warn:
+
+```
+Deprecated in 0.1.0, will be removed in 0.2.0. The old_property is deprecated.
Please use the something_else property instead.
+```
+
## Type annotations
For the type annotation the types from the `Typing` package are used.
diff --git a/mkdocs/docs/how-to-release.md b/mkdocs/docs/how-to-release.md
index ce38cbfc..59ad6977 100644
--- a/mkdocs/docs/how-to-release.md
+++ b/mkdocs/docs/how-to-release.md
@@ -38,6 +38,16 @@ For example, the API with the following deprecation tag
should be removed when p
)
```
+We also have the `deprecation_message` function. We need to change the
behavior according to what is noted in the message of that deprecation.
+
+```python
+deprecation_message(
+ deprecated_in="0.1.0",
+ removed_in="0.2.0",
+ help_message="The old_property is deprecated. Please use the
something_else property instead.",
+)
+```
+
## Running a release candidate
Make sure that the version is correct in `pyproject.toml` and
`pyiceberg/__init__.py`. Correct means that it reflects the version that you
want to release.
diff --git a/pyiceberg/catalog/__init__.py b/pyiceberg/catalog/__init__.py
index 02b9f9dd..6469c0d6 100644
--- a/pyiceberg/catalog/__init__.py
+++ b/pyiceberg/catalog/__init__.py
@@ -67,7 +67,7 @@ from pyiceberg.typedef import (
RecursiveDict,
)
from pyiceberg.utils.config import Config, merge_config
-from pyiceberg.utils.deprecated import deprecated
+from pyiceberg.utils.deprecated import deprecation_message
if TYPE_CHECKING:
import pyarrow as pa
@@ -742,11 +742,11 @@ class MetastoreCatalog(Catalog, ABC):
for property_name in DEPRECATED_PROPERTY_NAMES:
if self.properties.get(property_name):
- deprecated(
+ deprecation_message(
deprecated_in="0.7.0",
removed_in="0.8.0",
help_message=f"The property {property_name} is deprecated.
Please use properties that start with client., glue., and dynamo. instead",
- )(lambda: None)()
+ )
def create_table_transaction(
self,
diff --git a/pyiceberg/catalog/rest.py b/pyiceberg/catalog/rest.py
index 813398fa..639d732b 100644
--- a/pyiceberg/catalog/rest.py
+++ b/pyiceberg/catalog/rest.py
@@ -71,7 +71,8 @@ from pyiceberg.table.metadata import TableMetadata
from pyiceberg.table.sorting import UNSORTED_SORT_ORDER, SortOrder,
assign_fresh_sort_order_ids
from pyiceberg.typedef import EMPTY_DICT, UTF8, IcebergBaseModel, Identifier,
Properties
from pyiceberg.types import transform_dict_value_to_str
-from pyiceberg.utils.properties import property_as_bool
+from pyiceberg.utils.deprecated import deprecation_message
+from pyiceberg.utils.properties import get_first_property_value,
property_as_bool
if TYPE_CHECKING:
import pyarrow as pa
@@ -120,6 +121,7 @@ SIGV4 = "rest.sigv4-enabled"
SIGV4_REGION = "rest.signing-region"
SIGV4_SERVICE = "rest.signing-name"
AUTH_URL = "rest.authorization-url"
+OAUTH2_SERVER_URI = "oauth2-server-uri"
HEADER_PREFIX = "header."
NAMESPACE_SEPARATOR = b"\x1f".decode(UTF8)
@@ -291,11 +293,38 @@ class RestCatalog(Catalog):
@property
def auth_url(self) -> str:
- if url := self.properties.get(AUTH_URL):
+ if self.properties.get(AUTH_URL):
+ deprecation_message(
+ deprecated_in="0.8.0",
+ removed_in="0.9.0",
+ help_message=f"The property {AUTH_URL} is deprecated. Please
use {OAUTH2_SERVER_URI} instead",
+ )
+
+ self._warn_oauth_tokens_deprecation()
+
+ if url := get_first_property_value(self.properties, AUTH_URL,
OAUTH2_SERVER_URI):
return url
else:
return self.url(Endpoints.get_token, prefixed=False)
+ def _warn_oauth_tokens_deprecation(self) -> None:
+ has_oauth_server_uri = OAUTH2_SERVER_URI in self.properties
+ has_credential = CREDENTIAL in self.properties
+ has_init_token = TOKEN in self.properties
+ has_sigv4_enabled = property_as_bool(self.properties, SIGV4, False)
+
+ if not has_oauth_server_uri and (has_init_token or has_credential) and
not has_sigv4_enabled:
+ deprecation_message(
+ deprecated_in="0.8.0",
+ removed_in="1.0.0",
+ help_message="Iceberg REST client is missing the OAuth2 server
URI "
+ f"configuration and defaults to
{self.uri}{Endpoints.get_token}. "
+ "This automatic fallback will be removed in a future Iceberg
release."
+ f"It is recommended to configure the OAuth2 endpoint using the
'{OAUTH2_SERVER_URI}'"
+ "property to be prepared. This warning will disappear if the
OAuth2"
+ "endpoint is explicitly configured. See
https://github.com/apache/iceberg/issues/10537";,
+ )
+
def _extract_optional_oauth_params(self) -> Dict[str, str]:
optional_oauth_param = {SCOPE: self.properties.get(SCOPE) or
CATALOG_SCOPE}
set_of_optional_params = {AUDIENCE, RESOURCE}
diff --git a/pyiceberg/utils/deprecated.py b/pyiceberg/utils/deprecated.py
index 0de8cbfa..92051b4f 100644
--- a/pyiceberg/utils/deprecated.py
+++ b/pyiceberg/utils/deprecated.py
@@ -30,16 +30,33 @@ def deprecated(deprecated_in: str, removed_in: str,
help_message: Optional[str]
def decorator(func: Callable): # type: ignore
@functools.wraps(func)
def new_func(*args: Any, **kwargs: Any) -> Any:
- warnings.simplefilter("always", DeprecationWarning) # turn off
filter
-
- warnings.warn(
- f"Call to {func.__name__}, deprecated in {deprecated_in}, will
be removed in {removed_in}.{help_message}",
- category=DeprecationWarning,
- stacklevel=2,
- )
- warnings.simplefilter("default", DeprecationWarning) # reset
filter
+ message = f"Call to {func.__name__}, deprecated in
{deprecated_in}, will be removed in {removed_in}.{help_message}"
+
+ _deprecation_warning(message)
+
return func(*args, **kwargs)
return new_func
return decorator
+
+
+def deprecation_message(deprecated_in: str, removed_in: str, help_message:
Optional[str]) -> None:
+ """Mark properties or behaviors as deprecated.
+
+ Adding this will result in a warning being emitted.
+ """
+ message = f"Deprecated in {deprecated_in}, will be removed in
{removed_in}. {help_message}"
+
+ _deprecation_warning(message)
+
+
+def _deprecation_warning(message: str) -> None:
+ warnings.simplefilter("always", DeprecationWarning) # turn off filter
+
+ warnings.warn(
+ message,
+ category=DeprecationWarning,
+ stacklevel=2,
+ )
+ warnings.simplefilter("default", DeprecationWarning) # reset filter
diff --git a/tests/catalog/test_rest.py b/tests/catalog/test_rest.py
index fe9a32fe..174fff5d 100644
--- a/tests/catalog/test_rest.py
+++ b/tests/catalog/test_rest.py
@@ -24,7 +24,7 @@ from requests_mock import Mocker
import pyiceberg
from pyiceberg.catalog import PropertiesUpdateSummary, load_catalog
-from pyiceberg.catalog.rest import AUTH_URL, RestCatalog
+from pyiceberg.catalog.rest import OAUTH2_SERVER_URI, RestCatalog
from pyiceberg.exceptions import (
AuthorizationExpiredError,
NamespaceAlreadyExistsError,
@@ -235,7 +235,7 @@ def test_token_200_w_auth_url(rest_mock: Mocker) -> None:
)
# pylint: disable=W0212
assert (
- RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS,
**{AUTH_URL: TEST_AUTH_URL})._session.headers[
+ RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS,
**{OAUTH2_SERVER_URI: TEST_AUTH_URL})._session.headers[
"Authorization"
]
== f"Bearer {TEST_TOKEN}"
diff --git a/tests/utils/test_deprecated.py b/tests/utils/test_deprecated.py
index 7c44c458..a77ed665 100644
--- a/tests/utils/test_deprecated.py
+++ b/tests/utils/test_deprecated.py
@@ -35,3 +35,17 @@ def test_deprecated(warn: Mock) -> None:
assert warn.call_args[0] == (
"Call to deprecated_method, deprecated in 0.1.0, will be removed in
0.2.0. Please use load_something_else() instead.",
)
+
+
+@patch("warnings.warn")
+def test_deprecation_message(warn: Mock) -> None:
+ from pyiceberg.utils.deprecated import deprecation_message
+
+ deprecation_message(
+ deprecated_in="0.1.0",
+ removed_in="0.2.0",
+ help_message="Please use something_else instead",
+ )
+
+ assert warn.called
+ assert warn.call_args[0] == ("Deprecated in 0.1.0, will be removed in
0.2.0. Please use something_else instead",)
