search

meltano / sdk

Write 70% less code by using the SDK to build custom extractors and loaders that adhere to the Singer standard: https://sdk.meltano.com
https://sdk.meltano.com
Apache License 2.0
94 stars 66 forks source link

RESTStream: PreparedRequest - Be compatible with "standard" Session method behaviour #345

Closed MeltyBot closed 3 weeks ago

MeltyBot commented 2 years ago

Migrated from GitLab: https://gitlab.com/meltano/sdk/-/issues/347

Originally created by @cwegener on 2022-03-10 12:23:07

Summary

The requests library allows usage of certain environment variables to control its behaviour.

One of these environment variables is REQUESTS_CA_BUNDLE that is used to override the default CA bundle that ships with the requests library. Which is a needed feature in order to work with SSL-inspection devices and other 'man-in-the-middle' proxy systems.

As per requests documentation, in order to provide the same behaviour in regards to environment variables when using the PreparedRequest class instead of the Session class, additional preparation is needed via the merge_environment_settings API method, which provides the compatible behaviour.

Link

Proposed benefits

Provide the same behaviour to Meltano SDK users that is already afforded to users of the requests library.

Proposal details

Use merge_environment_settings API before calling send on the PreparedRequest object.

Best reasons not to build

Existing users who are relying on this unexpected behaviour (i.e. that Meltano RESTStream class is "ignoring" the otherwise respected REQUESTS_CA_BUNDLE, CURL_CA_BUNDLE, HTTP_PROXY, HTTPS_PROXY and NO_PROXY environment variables) might have their set ups broken by enabling this usually expected behaviour of the standard requests functionality.

MeltyBot commented 2 years ago

View 5 previous comments from the original issue on GitLab

stale[bot] commented 1 year ago

This has been marked as stale because it is unassigned, and has not had recent activity. It will be closed after 21 days if no further activity occurs. If this should never go stale, please add the evergreen label, or request that it be added.

edgarrmondragon commented 1 year ago

Still relevant

stale[bot] commented 1 month ago

This has been marked as stale because it is unassigned, and has not had recent activity. It will be closed after 21 days if no further activity occurs. If this should never go stale, please add the evergreen label, or request that it be added.