Skip to content
This repository has been archived by the owner on Mar 28, 2019. It is now read-only.

Implement batch api #93

Merged
merged 11 commits into from
Feb 6, 2015
Merged

Implement batch api #93

merged 11 commits into from
Feb 6, 2015

Conversation

leplatrem
Copy link
Contributor

@leplatrem
Copy link
Contributor Author

@Natim @tarekziade @ametaireau This is a first shot on the implementation of POST /batch. If you see something outrageously wrong, please speak up! :)

"body" : {
"title": "MoFo",
"url" : "http://mozilla.org"
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: update this to put body as string

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a requirements? It seems strange to me to have JSON encoded string inside a JSON. I rather prefer to have a nested object instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right. But we should support both then (suppose we want to support xml or form encoded payloads)

Copy link
Contributor

Choose a reason for hiding this comment

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

Well we don't :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I concur here: we currently don't and in case we want to, we can re-assess this formalism to include arbitrary strings.

@Natim Natim changed the title [WIP] implement batch api Implement batch api Feb 6, 2015

:note:

The responses are not necessarily in the same order of the requests.
Copy link
Contributor

Choose a reason for hiding this comment

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

Well it should. I doesn't make sense to have them in another order since they don't have ids how to you merge the results if there are not in the same order?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to leave this, in case batch requests are executed asynchronously. I can add a counter then :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Asynchronously doesn't means that the results should be shuffled. See https://github.com/caolan/async#maparr-iterator-callback

@leplatrem leplatrem force-pushed the 2-implement-batch-api branch from a350e58 to 6c11de7 Compare February 6, 2015 16:11
@leplatrem
Copy link
Contributor Author

@Natim r?

Natim added a commit that referenced this pull request Feb 6, 2015
@Natim Natim merged commit 4091059 into master Feb 6, 2015
@Natim Natim deleted the 2-implement-batch-api branch February 6, 2015 18:23

::

{
Copy link
Contributor

Choose a reason for hiding this comment

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

If we provide examples, we should do it with HTTPie so that it's easier for clients to reproduce.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be adressed in #101

for k, v in defaults.items():
if k == 'headers':
for i, j in v.items():
request.get(k, {}).setdefault(i, j)
Copy link
Contributor

Choose a reason for hiding this comment

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

I like that :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored in #98 :)

@leplatrem leplatrem modified the milestone: 0.2 Feb 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants