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

feat: Add ability to chain HTTP exports for multiple destinations #860

Merged

Conversation

lenny-goodell
Copy link
Member

@lenny-goodell lenny-goodell commented May 26, 2021

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What is the current behavior?

HTTP export returns HTTP response and stops pipeline on send errors. This block the ability to chain multiple HTTP exports in the pipeline to send the same data to multiple destinations.
Configurable pipeline only allows once instance of each function to be configured. Configuration names must match actual function names exactly. i.e. HttpExport matches HttpExport, but HttpExport2 does not match HttpExport
Issue Number: #256

What is the new behavior?

HTTP export now has option to return the input data and optionally continue pipeline on send errors. This enables the ability to chain multiple HTTP exports in the pipeline to send the same data to multiple destinations.
Configurable pipeline now supports multiple instance functions to be configured. Configuration names that start with the actual function name now match. i.e. HttpExport matches HttpExport and HttpExport2 also matches HttpExport.

Does this PR introduce a breaking change?

  • Yes
  • No

Are there any new imports or modules? If so, what are they used for and why?

no

Are there any specific instructions or things that should be known prior to reviewing?

Other information

@lenny-goodell lenny-goodell force-pushed the http-export-chaining branch 4 times, most recently from 820c102 to a149896 Compare May 28, 2021 00:28
Copy link
Contributor

@AlexCuse AlexCuse left a comment

Choose a reason for hiding this comment

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

LGTM

AlexCuse
AlexCuse previously approved these changes May 28, 2021
HTTP export now has option to return the input data and optionally continue pipeline on send errors.
This enables the ability to chain multiple HTTP exports in the pipeline to send the same data to multiple destinations.
Configurable pipeline now supports multiple instances of same function to be configured.
Configuration names that start with the actual function name now match.  i.e. `HttpExport` matches `HttpExport` and `HttpExport2` also matches `HttpExport`.

closes #256

Signed-off-by: lenny <leonard.goodell@intel.com>
@codecov-commenter
Copy link

Codecov Report

Merging #860 (cbd716a) into master (881afdc) will increase coverage by 0.78%.
The diff coverage is 90.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #860      +/-   ##
==========================================
+ Coverage   62.35%   63.14%   +0.78%     
==========================================
  Files          31       31              
  Lines        2048     2089      +41     
==========================================
+ Hits         1277     1319      +42     
+ Misses        679      678       -1     
  Partials       92       92              
Impacted Files Coverage Δ
internal/app/configupdates.go 11.36% <83.33%> (+11.36%) ⬆️
pkg/transforms/http.go 89.87% <83.33%> (+2.91%) ⬆️
internal/app/configurable.go 66.48% <100.00%> (+2.19%) ⬆️
internal/app/service.go 40.17% <100.00%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 881afdc...cbd716a. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple Export destinations
4 participants