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

Added possibility to switch between HTTP and HTTPS #26

Merged
merged 7 commits into from Oct 30, 2018

Conversation

dabrign
Copy link
Contributor

@dabrign dabrign commented May 22, 2018

In some case, e.g. installation on prem. Splunk is using http protocol instead of https.
This patch adds the possibility to select the correct protocol to push log on Splunk.
The default behaviour is not changed since by default https protocol is used

Added possibility to switch between http and https connection
Now is possible to use proxies configurations.
The proxies conf have to be defined in standard urllib3 way, e.g.
proxyes:{'http':'http://USER:PSWD@proxy.url:PORT', 'https':'https://USER:PSWD@proxy.url:PORT'}
In case use send a python dictionary to a _json type collector you can enable this feature to translate to a valid json for splunk
@zach-taylor
Copy link
Owner

Hey @dabrign thanks for the contribution! A couple things before I can consider this:

  • fix up the whitespace issues found by CodeClimate (don't worry about the complexity issues, I need to do some refactoring)
  • remove the merge commits

@stuehmer
Copy link

Hi, I think at least one occurrence of https was missed here: https://github.com/zach-taylor/splunk_handler/blob/master/splunk_handler/__init__.py#L196

@bdirito
Copy link

bdirito commented Sep 13, 2018

@stuehmer
No, that line got converted to

            url = '%s://%s:%s/services/collector' % (self.protocol,self.host, self.port)

@stuehmer
Copy link

@bdirito Great! (Sorry, I must have looked at the wrong diff).

@command-tab
Copy link

Any chance this will get merged and deployed to PyPi? This is the fix I'm looking for 😍

Thanks!

@dabrign
Copy link
Contributor Author

dabrign commented Oct 26, 2018

@zach-taylor
I just removed the white spaces, frankly I don't know how to remove the merge commits, can you help me on that?

@zach-taylor
Copy link
Owner

@dabrign I can also just squash merge this down to 1 commit on master, if you don't mind.

@dabrign
Copy link
Contributor Author

dabrign commented Oct 27, 2018

@zach-taylor fine to me! 👍

@zach-taylor zach-taylor merged commit c40483d into zach-taylor:master Oct 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants