-
Notifications
You must be signed in to change notification settings - Fork 10
feat: add dynamic url info #998
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
base: main
Are you sure you want to change the base?
Changes from all commits
891db6c
1ab8eeb
35228b4
fd8faf3
94a01a4
635660b
749c367
976248f
d0045f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| ### Breaking | ||
|
|
||
| ### Features | ||
| * generic_adder: add support for templated http urls | ||
|
|
||
| ### Improvements | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,10 +25,13 @@ | |||||||||||||||||
| """ | ||||||||||||||||||
|
|
||||||||||||||||||
| import typing | ||||||||||||||||||
| from typing import Sequence | ||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| from logprep.ng.abc.processor import Processor | ||||||||||||||||||
| from logprep.processor.base.exceptions import ProcessingWarning | ||||||||||||||||||
| from logprep.processor.base.rule import Rule | ||||||||||||||||||
| from logprep.processor.generic_adder.rule import GenericAdderRule | ||||||||||||||||||
| from logprep.util.getter import RefreshableGetter | ||||||||||||||||||
| from logprep.util.helper import add_fields_to | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -37,8 +40,26 @@ class GenericAdder(Processor): | |||||||||||||||||
|
|
||||||||||||||||||
| rule_class = GenericAdderRule | ||||||||||||||||||
|
|
||||||||||||||||||
| @property | ||||||||||||||||||
| def _rules(self) -> Sequence[GenericAdderRule]: | ||||||||||||||||||
| """Returns all rules""" | ||||||||||||||||||
| return typing.cast(Sequence[GenericAdderRule], self.rules) | ||||||||||||||||||
|
Comment on lines
+43
to
+46
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not "rules"?
Suggested change
like in and other sites |
||||||||||||||||||
|
|
||||||||||||||||||
| async def setup(self): | ||||||||||||||||||
| await super().setup() | ||||||||||||||||||
| for rule in self._rules: | ||||||||||||||||||
| rule.init_generic_adder(self._job_tag_for_cleanup) | ||||||||||||||||||
|
|
||||||||||||||||||
| def _apply_rules(self, event: dict, rule: Rule) -> None: | ||||||||||||||||||
| rule = typing.cast(GenericAdderRule, rule) | ||||||||||||||||||
| items_to_add = rule.add | ||||||||||||||||||
|
|
||||||||||||||||||
| try: | ||||||||||||||||||
| items_to_add = rule.add(event) | ||||||||||||||||||
| except Exception as error: | ||||||||||||||||||
| raise ProcessingWarning(str(error), rule, event) from error | ||||||||||||||||||
| if items_to_add: | ||||||||||||||||||
| add_fields_to(event, items_to_add, rule, rule.merge_with_target, rule.overwrite_target) | ||||||||||||||||||
|
|
||||||||||||||||||
| def _shut_down(self) -> None: | ||||||||||||||||||
| RefreshableGetter.remove_callbacks_for_tag(self._job_tag_for_cleanup) | ||||||||||||||||||
| return super()._shut_down() | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -25,10 +25,12 @@ | |||||
| """ | ||||||
|
|
||||||
| import typing | ||||||
| from typing import Sequence | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| from logprep.abc.processor import Processor | ||||||
| from logprep.processor.base.rule import Rule | ||||||
| from logprep.processor.base.exceptions import ProcessingWarning | ||||||
| from logprep.processor.generic_adder.rule import GenericAdderRule | ||||||
| from logprep.util.getter import RefreshableGetter | ||||||
| from logprep.util.helper import add_fields_to | ||||||
|
|
||||||
|
|
||||||
|
|
@@ -37,8 +39,24 @@ class GenericAdder(Processor): | |||||
|
|
||||||
| rule_class = GenericAdderRule | ||||||
|
|
||||||
| def _apply_rules(self, event: dict, rule: Rule): | ||||||
| rule = typing.cast(GenericAdderRule, rule) | ||||||
| items_to_add = rule.add | ||||||
| @property | ||||||
| def _rules(self) -> Sequence[GenericAdderRule]: | ||||||
| """Returns all rules""" | ||||||
| return typing.cast(Sequence[GenericAdderRule], self.rules) | ||||||
|
|
||||||
| def setup(self): | ||||||
| super().setup() | ||||||
| for rule in self._rules: | ||||||
| rule.init_generic_adder(self._job_tag_for_cleanup) | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we really need this external init and would like to avoid it. The
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes we do, I forgot to add the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mh, I see where you are coming from. Alright, we can keep this for consistency with other processors (namely But I still think that the Currently we are calling an init (which has setup-like semantics) on the rule but basically handle the shutdown on the processor-level, which is inconsistent. |
||||||
|
|
||||||
| def _apply_rules(self, event: dict, rule: GenericAdderRule): | ||||||
| try: | ||||||
| items_to_add = rule.add(event) | ||||||
| except Exception as error: | ||||||
| raise ProcessingWarning(str(error), rule, event) from error | ||||||
| if items_to_add: | ||||||
| add_fields_to(event, items_to_add, rule, rule.merge_with_target, rule.overwrite_target) | ||||||
|
|
||||||
| def _shut_down(self) -> None: | ||||||
| RefreshableGetter.remove_callbacks_for_tag(self._job_tag_for_cleanup) | ||||||
| return super()._shut_down() | ||||||
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.
Can you add this to
get_dictas well? Not related to your PR, but it would make sense to add support to all three high-level interface methods in the same way.Also, my personal preference would be:
(so without the line break)
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.
Sure ill fix the line breaks, but adding this to get_dict somehow feels wrong to me, because we just have to do this here and for collection because we want to return a list, but in the dict, we can just get the content_field value like you would normally in a dict. Whats your opinion on this?