-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Adding Tomcat plugin. #2447
Adding Tomcat plugin. #2447
Conversation
Any chance this will make the 1.3 release? |
No, I released a rc for 1.3 yesterday: #2733. |
Hi, Is this plugin release added to the next milestone? Which version will this plugin be available? |
We would like to test it!! |
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.
Looks great, can you fix one issue in the toml before I merge?
plugins/inputs/tomcat/README.md
Outdated
# Cedentials for status URI. | ||
# Default is tomcat/s3cret. | ||
username = tomcat | ||
password = s3cret |
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.
These strings need to be quoted so they are valid toml.
plugins/inputs/tomcat/README.md
Outdated
|
||
### Measurements & Fields: | ||
|
||
<optional description> |
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.
Remove this line
plugins/inputs/tomcat/tomcat.go
Outdated
## Cedentials for status URI. | ||
## Default is tomcat/s3cret. | ||
username = tomcat | ||
password = s3cret |
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.
Quote toml strings here too.
Quotes added. CircleCI complained about a zookeeper task. Not sure that's related to this pr.. I can follow-up if it is, though. |
I think if you rebase it will fix the zookeeper issue. |
I don't really see how a rebase would solve this CircleCI error:
Can you re-trigger the build to try it again? Permissions don't allow me to do that. |
Oh sorry, I spoke too soon. I didn't notice the |
I haven't tried that ever but when merge I will do a "Squash and merge" so as long as it does not introduce unwanted diffs it's fine. |
ok. I rebased off master, and tests now pass. The rebase introduced a few commits... but there hasn't been a tag or version release to use as a rebase point since the latest changes to the |
You will need to remove the unrelated commits during the rebase (use the |
Accidentally closed this while trying to get our fork in sync. |
This plugin resolves open issue #763 (#763)
Required for all PRs: