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

Respect path prefix in influx output uri #3224

Merged

Conversation

dimrozakis
Copy link
Contributor

@dimrozakis dimrozakis commented Sep 13, 2017

Fixes issues:

Duplicate of PR #2751, attempts to address #2751 (comment)

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@danielnelson
Copy link
Contributor

Can you also add a unittest?

@dimrozakis
Copy link
Contributor Author

@danielnelson I wasn't sure what kind of new unittest to add in order to properly test the path prefix support, so I chose running the TestHTTPClient_Write and TestHTTPClient_Query tests twice, with and without a path prefix. Are we ok with that?

@danielnelson
Copy link
Contributor

I would write a new testcase that only tests the path handling, it can do both the write and query operations. The handler just needs to test that the path is correct and that's it.

Basically reverts fb851ad.

Since path prefixes for http urls are now supported, this clarification
isn't needed.
@dimrozakis
Copy link
Contributor Author

@danielnelson I changed the tests as requested

@danielnelson danielnelson merged commit 9c8f4af into influxdata:master Sep 14, 2017
@danielnelson danielnelson added this to the 1.5.0 milestone Sep 14, 2017
@danielnelson danielnelson added area/influxdb feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin labels Sep 14, 2017
@dimrozakis dimrozakis deleted the influxdb-output-path-prefix branch September 14, 2017 00:41
danielnelson pushed a commit that referenced this pull request Oct 18, 2017
@danielnelson danielnelson modified the milestones: 1.5.0, 1.4.3 Oct 18, 2017
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/influxdb feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants