-
Notifications
You must be signed in to change notification settings - Fork 42
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
Feature bulk request #81
base: master
Are you sure you want to change the base?
Conversation
In case it helps: we've tried this today and it works like a charm :-) |
@@ -92,7 +92,7 @@ public function findDocumentById($id) | |||
* @param string $content | |||
* @return Response | |||
*/ | |||
public function request($method, $path = null, array $arguments = [], $content = null) | |||
public function request($method, $path = null, array $arguments = [], $content = null, $header = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please specify the the type of the $header
attribute and mention it in the commet
* @param string|array $content | ||
* @return Response | ||
*/ | ||
public function bulkRequest(array $arguments = [], $content = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return type missing
* @return Response | ||
*/ | ||
public function request($method, $path = null, array $arguments = [], $content = null) | ||
public function request($method, $path = null, array $arguments = [], $content = null, $header = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the header is a string, wouldn't be an empty string a better default?
|
||
$request->setHeader('Content-Type', 'application/json'); | ||
|
||
$header = $header ?: 'application/json'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't be 'application/json' a better default then?
Hey cwenzelg, thanks for the PR and sorry for the late review. Cheers, |
Hi @daniellienert , well this pull request ist really old. even before the return types etc. We are not working with Flow 6.0 so I don't know the changes that need to be done. we use our own branch till now and I would really love a backword merge with an older version so we can use it with the older versions, too. would that be possible. I can push a new commit that takes care about your comments. |
Adds a feature to push bulk requests. This is necessary if you want to create index tables based on thousands of entries via a command for example.