-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Enhanced support for batch endpoints #3042
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
Conversation
@flovilmart updated the pull request - view changes |
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.
Just a couple of nits
} | ||
|
||
function mBatchRoutingPath(originalUrl, serverURL, publicServerURL) { | ||
serverURL = serverURL ? parseURL(serverURL) : undefined; |
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.
minor but could you just move the check for URL being defined into parseURL? e.g. change in the function to if (URL && typeof URL === 'string') {
return undefined; | ||
} | ||
|
||
function mBatchRoutingPath(originalUrl, serverURL, publicServerURL) { |
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.
nit: is m short for make? Prefer explicit function names for clarity
if (requestPath.slice(0, apiPrefixLength) != apiPrefix) { | ||
throw new Parse.Error( | ||
Parse.Error.INVALID_JSON, | ||
'cannot route batch path ' + path); |
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.
I think path here will point to the require. Should be requestPath
?
|
||
let makeRoutablePath = function(requestPath) { | ||
// The routablePath is the path minus the api prefix | ||
if (requestPath.slice(0, apiPrefixLength) != apiPrefix) { |
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.
minor: could replace apiPrefixLength with apiPrefix.length here and on line 35 and then remove line 44
a30423d
to
e5f1bd0
Compare
@flovilmart updated the pull request - view changes |
* Allow to have different endpoint on publicserverURL and serverURL when batching * nits
* Allow to have different endpoint on publicserverURL and serverURL when batching * nits
Adds ability to have publicServerURL different from serverURL ex:
http://api.my-parse.com/
andhttp://localhost/parse
for batch callsFixes #2980