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

vdk-ingest-http: additional request parameters support #701

Merged
merged 4 commits into from
Feb 11, 2022

Conversation

ivakoleva
Copy link
Contributor

@ivakoleva ivakoleva commented Feb 10, 2022

vdk-ingest-http: additional request parameters support

Plugin was missing connect & read timeout configurations,
along with cert file path, and configurable TLS certificate
verification that was hardcoded to False (tagged with nosec).
It was also missing retries configuration on total retries count,
backoff factor, and status forcelist.

Added configurations for INGEST_OVER_HTTP_CONNECT_TIMEOUT_SECONDS,
INGEST_OVER_HTTP_READ_TIMEOUT_SECONDS, INGEST_OVER_HTTP_CERT_FILE_PATH,
and INGEST_OVER_HTTP_VERIFY that is enabled by default.
Added also INGEST_OVER_HTTP_RETRY_TOTAL,
INGEST_OVER_HTTP_RETRY_BACKOFF_FACTOR and
INGEST_OVER_HTTP_RETRY_STATUS_FORCELIST configurations.

Testing Done: added test_ingest_over_http_request_parameters_propagation
unit test.

Signed-off-by: ikoleva [email protected]

Plugin was missing connect & read timeout configurations,
along with cert file path, and configurable TLS certificate
verification that was hardcoded to False (tagged with nosec).

Added configurations for INGEST_OVER_HTTP_CONNECT_TIMEOUT_SECONDS,
INGEST_OVER_HTTP_READ_TIMEOUT_SECONDS, INGEST_OVER_HTTP_CERT_FILE_PATH,
and INGEST_OVER_HTTP_VERIFY that is enabled by default.

Testing Done: added test_ingest_over_http_request_parameters_propagation
unit test.

Signed-off-by: ikoleva <[email protected]>
Copy link
Contributor

@doks5 doks5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks ok to me.

Plugin was missing connect & read timeout configurations,
along with cert file path, and configurable TLS certificate
verification that was hardcoded to False (tagged with nosec).
It was also missing retries configuration on total retries count,
backoff factor, and status forcelist.

Added configurations for INGEST_OVER_HTTP_CONNECT_TIMEOUT_SECONDS,
INGEST_OVER_HTTP_READ_TIMEOUT_SECONDS, INGEST_OVER_HTTP_CERT_FILE_PATH,
and INGEST_OVER_HTTP_VERIFY that is enabled by default.
Added also INGEST_OVER_HTTP_RETRY_TOTAL,
INGEST_OVER_HTTP_RETRY_BACKOFF_FACTOR and
INGEST_OVER_HTTP_RETRY_STATUS_FORCELIST configurations.

Testing Done: added test_ingest_over_http_request_parameters_propagation
unit test.

Signed-off-by: ikoleva <[email protected]>
@ivakoleva ivakoleva requested a review from doks5 February 10, 2022 19:47
Plugin was missing connect & read timeout configurations,
along with cert file path, and configurable TLS certificate
verification that was hardcoded to False (tagged with nosec).
It was also missing retries configuration on total retries count,
backoff factor, and status forcelist.

Added configurations for INGEST_OVER_HTTP_CONNECT_TIMEOUT_SECONDS,
INGEST_OVER_HTTP_READ_TIMEOUT_SECONDS, INGEST_OVER_HTTP_CERT_FILE_PATH,
and INGEST_OVER_HTTP_VERIFY that is enabled by default.
Added also INGEST_OVER_HTTP_RETRY_TOTAL,
INGEST_OVER_HTTP_RETRY_BACKOFF_FACTOR and
INGEST_OVER_HTTP_RETRY_STATUS_FORCELIST configurations.

Testing Done: added test_ingest_over_http_request_parameters_propagation
unit test.

Signed-off-by: ikoleva <[email protected]>
@ivakoleva ivakoleva enabled auto-merge (squash) February 11, 2022 10:57
@ivakoleva ivakoleva merged commit a876a81 into main Feb 11, 2022
@ivakoleva ivakoleva deleted the person/ikoleva/http-ingest-request-parameters branch February 11, 2022 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants