Draft: Fix: Use merge_environment_settings in RESTStream - [opened]

[MeltyBot]

Merges requests-preparedrequest-mergeenvironmentsettings -> main

Migrated from GitLab: https://gitlab.com/meltano/sdk/-/merge_requests/256

GitHub Issue: #345

[cwegener]

Hi guys.

I'd like to revisit this topic.

As I start using more SDK based RESTStream taps from the Meltano Hub, I now find myself having to:

1. clone the tap
2. update the dependency for singer-sdk to my fork with the merge_environment_settings method
3. build the tap
4. install my build of the tap

@edgarrmondragon You had some comments in the original Gitlab issue discussion about making merge_environment_settings configurable by the user.

Having a bit more meltano experience under my belt now after a few months, I understand the meltano approach a bit better.

Those changes that you suggested do make sense to me now.

I'll test out if the get_request_settings() logic that you proposed will work the way a meltano user expects them to work, which would be:

1. If either http_proxy or https_proxy env vars are present, the SDK based tap's Session.send() method will respect them (btw. I've had some anecdotal experience from a colleague that even without merge_environment_settings, Session.send() already seems to use http_proxy and https_proxy which seems to be by design .. see [1])
2. If neither http_proxy nor https_proxy are set, the SDK based tap's Session.send() method will not use any user-configured proxies. (transparent intercepting middleman proxies will obviously still intercept the HTTP requests)
3. If either REQUESTS_CA_BUNDLE or CURL_CA_BUNDLE env vars are present, the SDK based tap's Session.send() method will respect them and TLS inspection of any middleman proxies will function correctly.
4. It should never be possible to disable TLS verification through configuration options. This is not a feature that the requests library or libcurl provides. And Meltano SDK should not provide it either.

[1] psf/requests#6108

[edgarrmondragon]

I'll test out if the get_request_settings() logic that you proposed will work the way a meltano user expects them to work

Nice. Thanks for revisiting @cwegener!

[edgarrmondragon]

This could also be better addressed by #1649