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

Allow user to provide Guzzle options in requests #299

Merged
merged 4 commits into from Oct 28, 2020

Conversation

chapa
Copy link
Contributor

@chapa chapa commented Feb 22, 2020

Related to #292

@coveralls
Copy link

coveralls commented Feb 22, 2020

Pull Request Test Coverage Report for Build 690

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.004%) to 96.196%

Totals Coverage Status
Change from base Build 688: 0.004%
Covered Lines: 1669
Relevant Lines: 1735

💛 - Coveralls

@chapa
Copy link
Contributor Author

chapa commented Feb 22, 2020

Arf, just realized it could be better to name the option requestOptions instead of guzzle (like I suggested in #292) to be consistent with OpenStack::__construct().
What do you think @haphan ?

@giulioprovasi
Copy link

hey, any eta on this ? have same issue and is kinda blocking the usage of this package for me :/

@haphan
Copy link
Collaborator

haphan commented Oct 11, 2020

Arf, just realized it could be better to name the option requestOptions instead of guzzle (like I suggested in #292) to be consistent with OpenStack::__construct().
What do you think @haphan ?

Thanks @chapa. I think it makes a lot of sense to re-use the key requestOptions. Can you help to update the PR?

I'm also thinking to have an integration test case to cover this new feature. I can help with integration test if you don't have time.

@chapa
Copy link
Contributor Author

chapa commented Oct 11, 2020

Hey, I'm on vacation right now but I'll take a look when I get back.

If you can do an integration test it would be great !

@chapa
Copy link
Contributor Author

chapa commented Oct 26, 2020

Just pushed the commit to rename "guzzle' to 'requestOptions'.

I'm sorry but I don't have the time to dig into integration tests, I didn't even manage to run them because of this error :

$ php ./tests/integration/run.php -s=ObjectStore
INFO: =================================================
INFO: Starting objectstore v1 core integration test(s)
INFO: =================================================
PHP Fatal error:  Uncaught RuntimeException: OpenStack\Integration\Objectstore\v1\CoreTest does not exist in [...]/tests/integration/Runner.php:115
[...]

Looks like an autoloading problem, but composer.json seems fine and the autoload.php is required in run.php so I don't understand, I didn't look any further

@haphan
Copy link
Collaborator

haphan commented Oct 27, 2020

Cool. I will run integration tests on my end.

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

4 participants