-
Notifications
You must be signed in to change notification settings - Fork 28
feat: include "self" parameter in REST handler helper functions #1768
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: develop
Are you sure you want to change the base?
Conversation
Code Coverage 🎉
|
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.
lgtm, just one question
@@ -2,6 +2,7 @@ | |||
|
|||
import json | |||
import sys | |||
import inspect |
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.
Do you need this inspect here?
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.
We would have to make changes to input.module-template
file also.
@@ -62,6 +63,16 @@ class {{class_name}}(smi.Script): | |||
ew.write_event(event) | |||
{%- endif %} | |||
|
|||
{% if input_helper_module -%} | |||
def call_with_args(self, method, *args): |
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.
The drawback with this implementation is - when the add-on has this code and is deployed in Splunk environment and when the modinput is invoked, it will check whether the input_helper module has 2 parameters or 3, and then execute it.
I think, we could do this same during the ucc-gen build
process itself and write this file accordingly. Perhaps, something like this. The drawback here is the *pyc
that would be generated, which we would have to clean-up.
What do you think?
Issue number: ADDON-80774
PR Type
What kind of change does this PR introduce?
Summary
Changes
This PR introduces an additional and alternative way of specifying helper methods.
Previously:
Now, there is also another way (with
self
):User experience
Currently used helper modules will work as before.
Checklist
If an item doesn't apply to your changes, leave it unchecked.
Review
Tests
See the testing doc.
Demo/meeting:
Reviewers are encouraged to request meetings or demos if any part of the change is unclear