-
Notifications
You must be signed in to change notification settings - Fork 33
multipipe #83
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
multipipe #83
Conversation
|
The current state of the code does not allow command-line parameters. With multiple repeating stages in any order, we need to be explicit as to which stage (with its parameters) comes after which other stage. Let's discuss what command-line parameters can still be provided. |
dfbb9f0 to
8fc1a76
Compare
|
Revised format of the yaml file: I updated all the tests to work with the new format. |
8fc1a76 to
48418b6
Compare
|
Refactored some code and fixed the tests for write-loki. Updated the config format to remove redundancy. New format: |
Codecov Report
@@ Coverage Diff @@
## main #83 +/- ##
==========================================
- Coverage 56.05% 54.43% -1.63%
==========================================
Files 45 46 +1
Lines 2501 2662 +161
==========================================
+ Hits 1402 1449 +47
- Misses 1016 1118 +102
- Partials 83 95 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
playground/prom1.yml
Outdated
| - name: write1 | ||
| write: | ||
| type: stdout | ||
| >>>>>>> implemented multi-pipe and ported tests; Loki not working properly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line seems to have been leaked from a merge resolution.
ronensc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR includes many changes, so the review has many comments too...
A general note, I've seen many variable names with json in their name while their actual type is a map[string]interface{}.
Personally, I wouldn't include json in their name because they are in an internal format, not in json format.
pkg/pipeline/utils/params_parse.go
Outdated
| log "github.com/sirupsen/logrus" | ||
| ) | ||
|
|
||
| // for specified params structure, return its corresponding (json) string from config.parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto
|
@KalmanMeth you probably know but just to make sure, in the Conversation tab, GitHub hides 15 of my comments because there are too many. You need to click "Load more" to view them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KalmanMeth -- I am missing the api.md file. It should be updated based on all the changes and improvements you did to the API
|
@KalmanMeththe BTW: you need to execute |
I missed them earlier. Now they are addressed. |
eranra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me :-)
Took some inspiration from the go-pipes.
Allow the same
ingestto be directed to multiple pipelines.We allow a single
ingestand a singledecodestage.After that, we allow any order of
transform,extract,encode,writestages.Sample syntax:
This example shows 2 generic transforms receiving the same flows from the decoder and performing different transforms and different write with them.