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

introduce HTTParty-configuration, fix #61 #67

Merged
merged 1 commit into from
Sep 30, 2014

Conversation

barraq
Copy link
Contributor

@barraq barraq commented Aug 6, 2014

This commit introduces a new option attribute, namely httparty, that allows passing configuration down to HTTParty.
For instance to disable verification of self-signed certificates you can now do as follow:

client1 = Gitlab.client(endpoint: 'https://api1.example.com', private_token: 'user-001', httparty: {verify: false})

The list of HTTParty configuration attributes can be found at https://github.com/jnunemaker/httparty

This commit introduces a new option attribute, namely `httparty`, that allows passing configuration down to HTTParty.
For instance to disable verification of self-signed certificates you can now do as follow:

    client1 = Gitlab.client(endpoint: 'https://api1.example.com', private_token: 'user-001', httparty: {verify: false})

The list of HTTParty configuration attributes can be found at https://github.com/jnunemaker/httparty
@barraq
Copy link
Contributor Author

barraq commented Sep 30, 2014

@jvanbaarsen @NARKOZ ?

jvanbaarsen added a commit that referenced this pull request Sep 30, 2014
introduce HTTParty-configuration, fix #61
@jvanbaarsen jvanbaarsen merged commit e9107e0 into NARKOZ:master Sep 30, 2014
@jvanbaarsen
Copy link
Contributor

@barraq Thanks!

@dblessing
Copy link
Contributor

I think this is unnecessary. You can already pass any httparty config down via options hash. So, you would say 'options[:verify] = false' right?

The piece that allows setting a default set of options would be nice, but that portion of this pull is incomplete.

@jvanbaarsen
Copy link
Contributor

@dblessing Ok, what do you suggest? Complete this PR, or revert the PR?

@barraq
Copy link
Contributor Author

barraq commented Oct 1, 2014

@dblessing are you sure about that? I first tried as you suggested but it appeared that options were not passed along to httparty or that they were ignored...

@dblessing
Copy link
Contributor

Let me test and see.

@dblessing
Copy link
Contributor

Oh, I see the issue. The options hash is not consistently implemented in this library. If it was passed directly, and not reassigned, it would work as expected :)

In some places the options hash is assigned to :query like this:
https://github.com/NARKOZ/gitlab/blob/master/lib/gitlab/client/users.rb#L14

Given this, I think we have to keep this change. We could make things consistent but that would undoubtedly break what people are already using.

@barraq Even with the changes you made, I would caution you that not all methods even bother to accept options. -- But I guess that's where the default options come in.

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.

3 participants