Skip to content
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

Export function template #11923

Merged
merged 13 commits into from
May 30, 2019
Merged

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Apr 24, 2019

A new subcommand is added under export named function. This command prints the loadable template to stdout for enabled functions.

$ ./functionbeat export function cloudwatch

I extracted generating templates from CLIManager. Now it has a TemplateBuilder which constructs the template for the manager. A template builder can be retrieved from a provider, as generating templates is specific to providers. Thus, CLIManager does not need to know about the provider anymore.

TODO

  • more testing
  • changelog entry
  • better PR description

@kvch kvch force-pushed the feature-functionbeat-export-function branch 2 times, most recently from e58c3c2 to ff862ec Compare May 6, 2019 14:45
Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested it, but code looks good nice refactoring @kvch, could we add a simple python system test to make sure it work?

@kvch kvch requested a review from a team as a code owner May 7, 2019 10:03
@kvch kvch force-pushed the feature-functionbeat-export-function branch from a5228d7 to fa6ee74 Compare May 7, 2019 10:12
@ph
Copy link
Contributor

ph commented May 7, 2019

File "/usr/share/python-wheels/urllib3-1.19.1-py2.py3-none-any.whl/urllib3/util/retry.py", line 315, in increment
total -= 1
TypeError: unsupported operand type(s) for -=: 'Retry' and 'int'
../..//libbeat/scripts/Makefile:254: recipe for target 'python-env' failed

The above is weird, It's not in our code it's in the python library that we use.
I've rekicked the job on travis.

@ph
Copy link
Contributor

ph commented May 7, 2019

I've tested the export function cloudwatch and the document produced appear to be complete and valid, I have added comment concerning the usage and the help provided by that command.

@kvch
Copy link
Contributor Author

kvch commented May 23, 2019

jenkins test this

@kvch kvch force-pushed the feature-functionbeat-export-function branch from 9565ae5 to 7086fab Compare May 23, 2019 10:23
Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added a few minors comments concerning missing godoc and also a suggestion for the changelog, otherwise everything else LGTM.

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
@ph
Copy link
Contributor

ph commented May 23, 2019

Failure appears to be related to the changes in this PR.

@kvch
Copy link
Contributor Author

kvch commented May 30, 2019

Failing tests are unrelated.

@kvch kvch merged commit e91fd2b into elastic:master May 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants