vdk-core: Introduce Ingestion Plugin execution sequence interfaces #579
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
doks5
force-pushed
the
person/andonova/ingest-plugin-chaining
branch
from
f85da55
to
fe4f077
Compare
|
Can you link the issue ? |
ivakoleva
reviewed
Nov 30, 2021
projects/vdk-core/src/vdk/internal/builtin_plugins/ingestion/ingester_configuration_plugin.py
Outdated
Show resolved
Hide resolved
Done, thanks |
projects/vdk-core/src/vdk/internal/builtin_plugins/ingestion/ingester_configuration_plugin.py
Outdated
Show resolved
Hide resolved
doks5
force-pushed
the
person/andonova/ingest-plugin-chaining
branch
from
fe4f077
to
d74f504
Compare
antoniivanov
reviewed
Dec 1, 2021
projects/vdk-core/src/vdk/internal/builtin_plugins/ingestion/ingester_configuration_plugin.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Personally I would really benefit from seeing an actual implementation example as part of this PR, either as actual code or just a scenario inside the PR description. Right now my understanding of what the goal here is and how it's being achieved is very hazy.
projects/vdk-core/src/vdk/internal/builtin_plugins/ingestion/ingester_configuration_plugin.py
Outdated
Show resolved
Hide resolved
|
I totally agree with @gageorgiev. @doks5 could you please add examples and use cases. I am not sure what problem we are trying to solve. |
doks5
force-pushed
the
person/andonova/ingest-plugin-chaining
branch
2 times, most recently
from
8f6fa65
to
169d092
Compare
I've added workflow examples as pydoc to the interface. |
ivakoleva
requested changes
Dec 3, 2021
antoniivanov
reviewed
Dec 3, 2021
mivanov1988
reviewed
Dec 7, 2021
doks5
force-pushed
the
person/andonova/ingest-plugin-chaining
branch
5 times, most recently
from
360899e
to
b81c794
Compare
antoniivanov
approved these changes
Dec 7, 2021
projects/vdk-core/tests/functional/ingestion/test_ingest_chain.py
Outdated
Show resolved
Hide resolved
doks5
force-pushed
the
person/andonova/ingest-plugin-chaining
branch
from
b81c794
to
a9a466d
Compare
As part of the improvements to the Ingestion API, it is necessary to make it possible for multiple ingestion plugins to be chained and provide capability for complex payload processing. This change introduces new interface for the IngesterPlugin to facilitate payload processing without ingesting, and a new environment variable in the ingestion configuration to allow clients to specify the sequence in which ingestion plugins get executed. Testing Done: Added tests that would be used when the implementation of the interfaces is complete. Signed-off-by: Andon Andonov <andonova@vmware.com>
doks5
force-pushed
the
person/andonova/ingest-plugin-chaining
branch
from
a9a466d
to
968284c
Compare
ivakoleva
reviewed
Dec 8, 2021
projects/vdk-core/src/vdk/internal/builtin_plugins/ingestion/ingester_configuration_plugin.py
Outdated
Show resolved
Hide resolved
ivakoleva
reviewed
Dec 9, 2021
doks5
force-pushed
the
person/andonova/ingest-plugin-chaining
branch
from
4afc373
to
14bd0ae
Compare
There is one linked issue. Is there another one that needs to be linked? |
doks5
force-pushed
the
person/andonova/ingest-plugin-chaining
branch
from
14bd0ae
to
27751a6
Compare
ivakoleva
reviewed
Dec 9, 2021
projects/vdk-core/tests/functional/ingestion/test_ingest_chain.py
Outdated
Show resolved
Hide resolved
As part of the improvements to the Ingestion API, it is necessary to make it possible for multiple ingestion plugins to be chained and provide capability for complex payload processing. This change introduces new interface for the IngesterPlugin to facilitate payload processing without ingesting, and a new environment variable in the ingestion configuration to allow clients to specify the sequence in which ingestion plugins get executed. Testing Done: Added tests that would be used when the implementation of the interfaces is complete. Signed-off-by: Andon Andonov <andonova@vmware.com>
doks5
force-pushed
the
person/andonova/ingest-plugin-chaining
branch
from
27751a6
to
b7a1668
Compare
ivakoleva
reviewed
Dec 10, 2021
projects/vdk-core/tests/functional/ingestion/test_ingest_chain.py
Outdated
Show resolved
Hide resolved
ivakoleva
requested changes
Dec 10, 2021
… sequence interfaces As part of the improvements to the Ingestion API, it is necessary to make it possible for multiple ingestion plugins to be chained and provide capability for complex payload processing. This change introduces new interface for the IngesterPlugin to facilitate payload processing without ingesting, and a new environment variable in the ingestion configuration to allow clients to specify the sequence in which ingestion plugins get executed. Testing Done: Added tests that would be used when the implementation of the interfaces is complete. Signed-off-by: Andon Andonov <andonova@vmware.com>
ivakoleva
approved these changes
Dec 13, 2021
There was a problem hiding this comment.
Great, thanks for elaborating thoroughly, this change of interface surface that adds support for ingestion pre- and post- processing along with metadata, is fundamental
A small note, I would suggest using IngestionMetadata consistently in favour of swapping to IngestionResult. It seems simpler, and no copying needs to be implemented, and described as expected behaviour.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As part of the improvements to the Ingestion API, it is necessary to make it
possible for multiple ingestion plugins to be chained and provide capability for
complex payload processing.
This change introduces new interface for the IngesterPlugin to facilitate payload
processing without ingesting, and a new environment variable in the ingestion configuration
to allow clients to specify the sequence in which ingestion plugins get executed.
Testing Done: Trivial, documentation change mainly.
Signed-off-by: Andon Andonov andonova@vmware.com