Skip to content

Conversation

@SyZn
Copy link

@SyZn SyZn commented Apr 24, 2021

Added needed dependency

Since the Client is used in src/Source/AbstractSource.php as a Client this should be a prod dependency.

SyZn added 7 commits April 24, 2021 12:08
Added needed dependency
Added the parameter HTTPOptions to configure the GuzzleClient.
Fixed the error handling to report better errors, when there is no response to the request.
@SyZn
Copy link
Author

SyZn commented Apr 24, 2021

I added a way to prevent GuzzleClient to check SSL verification, because I had to access an API that had no valid SSL anymore (the API wil be closed soon - Contao will replace it, so renewing the cert does not make sense anymore)

@SyZn SyZn requested a review from Defcon0 April 26, 2021 22:13
Copy link
Contributor

@Defcon0 Defcon0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your efforts. I guess after these changes we have it ;-)

$options['verify'] = false;
}

$event = $this->eventDispatcher->dispatch(BeforeAuthenticationEvent::NAME, new BeforeAuthenticationEvent($auth, $this->sourceModel));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please replace auth by options and fix the getter and setter; Afterwards please pass $event->getOptions() to storeValueToRemoteCache() and getContentFromUrl(). Then I guess we have it :-) Thanks for your support!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please rename the event to ModifySourceHttpClientOptionsEvent since this makes more sense. This way we can modify all options of guzzle and not only auth settings.

}

protected function getContentFromUrl(string $method, string $url, array $auth = [], array $HTTPClientOptions = []): array
protected function getContentFromUrl(string $method, string $url, array $options = []): array
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename $options to $httpClientOptions since we might have a general $options array sometimes. (same below)

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.

2 participants