Skip to content

Commit

Permalink
fix(taps): Do not emit a warning needlessly when rest_method is not…
Browse files Browse the repository at this point in the history
… set in a stream class (#2870)

* fix(taps): Do not emit a warning needlessly when `rest_method` is not set in a stream class

* Make mypy happy
  • Loading branch information
edgarrmondragon authored Feb 5, 2025
1 parent 1d00bfc commit 61c1413
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 40 deletions.
2 changes: 1 addition & 1 deletion singer_sdk/streams/graphql.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class GraphQLStream(RESTStream, t.Generic[_TToken], metaclass=abc.ABCMeta):
"""

path = ""
rest_method = "POST"
http_method = "POST"

@classproperty
def records_jsonpath(cls) -> str: # type: ignore[override] # noqa: N805
Expand Down
48 changes: 12 additions & 36 deletions singer_sdk/streams/rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import copy
import decimal
import logging
import sys
import typing as t
from functools import cached_property
from http import HTTPStatus
Expand All @@ -29,11 +28,6 @@
)
from singer_sdk.streams.core import Stream

if sys.version_info < (3, 13):
from typing_extensions import deprecated
else:
from warnings import deprecated # pragma: no cover

if t.TYPE_CHECKING:
from collections.abc import Iterable, Mapping
from datetime import datetime
Expand Down Expand Up @@ -144,39 +138,21 @@ def get_url(self, context: Context | None) -> str:

# HTTP Request functions

@property
@deprecated(
"Use `http_method` instead.",
category=SingerSDKDeprecationWarning,
)
def rest_method(self) -> str:
"""HTTP method to use for requests. Defaults to "GET".
.. deprecated:: 0.43.0
Override :meth:`~singer_sdk.RESTStream.http_method` instead.
"""
return self._http_method or "GET"

@rest_method.setter
@deprecated(
"Use `http_method` instead.",
category=SingerSDKDeprecationWarning,
)
def rest_method(self, value: str) -> None:
"""Set the HTTP method for requests.
Args:
value: The HTTP method to use for requests.
.. deprecated:: 0.43.0
Override :meth:`~singer_sdk.RESTStream.http_method` instead.
"""
self._http_method = value

@property
def http_method(self) -> str:
"""HTTP method to use for requests. Defaults to "GET"."""
return self._http_method or self.rest_method
if self._http_method:
return self._http_method

if hasattr(self, "rest_method"):
warn(
"Use `http_method` instead.",
SingerSDKDeprecationWarning,
stacklevel=2,
)
return self.rest_method # type: ignore[no-any-return]

return "GET"

@http_method.setter
def http_method(self, value: str) -> None:
Expand Down
17 changes: 14 additions & 3 deletions tests/core/test_streams.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import logging
import typing as t
import urllib.parse
import warnings

import pytest
import requests
Expand Down Expand Up @@ -589,6 +590,10 @@ def callback(request: requests.PreparedRequest, context: requests_mock.Context):
context.reason = "Method Not Allowed"
return {"error": "Check your method"}

with warnings.catch_warnings():
warnings.simplefilter("error")
assert RestTestStream(tap).http_method == "GET"

class PostStream(RestTestStream):
records_jsonpath = "$.data[*]"
path = "/endpoint"
Expand All @@ -603,10 +608,16 @@ class PostStream(RestTestStream):
with pytest.raises(FatalAPIError, match="Method Not Allowed"):
list(stream.request_records(None))

with pytest.warns(SingerSDKDeprecationWarning):
stream.rest_method = "GET"
assert hasattr(stream, "http_method")
assert not hasattr(stream, "rest_method")

with pytest.raises(FatalAPIError, match="Method Not Allowed"):
stream.http_method = None
stream.rest_method = "GET"

with (
pytest.raises(FatalAPIError, match="Method Not Allowed"),
pytest.warns(SingerSDKDeprecationWarning),
):
list(stream.request_records(None))

stream.http_method = "POST"
Expand Down

0 comments on commit 61c1413

Please sign in to comment.