Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Use http_method instead warning despite not declaring rest_method #2869

Closed
1 task done
ReubenFrankel opened this issue Feb 5, 2025 · 4 comments · Fixed by #2870
Closed
1 task done

bug: Use http_method instead warning despite not declaring rest_method #2869

ReubenFrankel opened this issue Feb 5, 2025 · 4 comments · Fixed by #2870
Assignees
Labels
HTTP HTTP based taps and targets such (REST, XML, etc.)
Milestone

Comments

@ReubenFrankel
Copy link
Contributor

ReubenFrankel commented Feb 5, 2025

Singer SDK Version

0.44.2

Is this a regression?

  • Yes Sort of

Python Version

NA

Bug scope

Taps (catalog, state, etc.)

Operating System

No response

Description

tap-f1 does not declare rest_method anywhere (assumes default of GET) and yet there are loads of Warning: Use `http_method` instead. logs from Pytest: https://github.com/ReubenFrankel/tap-f1/actions/runs/13155539363/job/36711708911?pr=104#step:7:20

This looks to be the case if http_method is not declared (i.e. None) and the SDK resolves the default GET via internal call to rest_method:

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

@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"

Link to Slack/Linen

No response

@ReubenFrankel ReubenFrankel changed the title bug: `Use http_method instead warning despite not declaring rest_method` bug: Use http_method instead warning despite not declaring rest_method Feb 5, 2025
@ReubenFrankel ReubenFrankel changed the title bug: Use http_method instead warning despite not declaring rest_method bug: Use http_method instead warning despite not declaring rest_method Feb 5, 2025
@edgarrmondragon edgarrmondragon added the HTTP HTTP based taps and targets such (REST, XML, etc.) label Feb 5, 2025
@edgarrmondragon edgarrmondragon added this to the v0.44 milestone Feb 5, 2025
@edgarrmondragon
Copy link
Collaborator

Hey @ReubenFrankel 👋

Can you check if #2870 addresses this for you?

@ReubenFrankel
Copy link
Contributor Author

Hey @ReubenFrankel 👋

Can you check if #2870 addresses this for you?

Fixed 🙂

@edgarrmondragon
Copy link
Collaborator

Closed by #2870.

@ReubenFrankel
Copy link
Contributor Author

Lovely: https://github.com/ReubenFrankel/tap-f1/actions/runs/13162254699 Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HTTP HTTP based taps and targets such (REST, XML, etc.)
Projects
None yet
2 participants