-
Notifications
You must be signed in to change notification settings - Fork 385
Bugfix/spl 142009 #174
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
Bugfix/spl 142009 #174
Conversation
…eventing', whereas it should have been 'type=events', which is in consistency with splunk code.
| installedApps = [] | ||
|
|
||
| def assertEventuallyTrue(self, predicate, timeout=10, pause_time=0.5, | ||
| def assertEventuallyTrue(self, predicate, timeout=30, pause_time=0.5, |
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.
Why are we changing the default timeout for all tests?
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.
@itay this is change in the develop branch.
| | streams | streaming=True[,local=[True|False]] | type='streaming'[,distributed=[true|false] | | ||
| +----------+-------------------------------------+--------------------------------------------+ | ||
| | events | retainsevents=True, streaming=False | type='eventing' | | ||
| | events | retainsevents=True, streaming=False | type='events' | |
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.
Was this just a typo on our end?
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.
Nope..not a type..we were actually expecting 'eventing', whereas in splunk code we were expecting 'events'.
| - SPLUNK_VERSION=6.2.6-sdk | ||
| - SPLUNK_VERSION=6.3.1-sdk | ||
| - SPLUNK_VERSION=6.4-sdk | ||
| - SPLUNK_VERSION=6.5-sdk |
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.
@shakeelmohamed do we have these versions prepped on the Docker side?
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.
Yeah but they shouldn't be in this PR since they're on develop. @gaurav22gupta can you re-target this PR to the develop branch?
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.
@itay @shakeelmohamed Done. Changed the base to develop.
|
@gaurav22gupta looks like you need to update some tests: https://travis-ci.org/splunk/splunk-sdk-python/jobs/242162504 Look at the bottom to see which ones failed. |
|
@itay @shakeelmohamed At present, failing test cases are unrelated to this PR. Previously failing test cases have passed now. |
|
@shakeelmohamed this LGTM - you can merge once you're ready. |
Changed the SDK for python to accept generating command type as 'events' instead of 'eventing', for consistency with splunk core.