-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
update function and freeze schema #97
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
…tionRequest specs
As discused in #90 :) |
@natanrolnik If you use Parse JS Sdk, you will get installationId by default, on every request, as it is sent on body payload :
|
Cool. I'll check regarding the iOS SDK. Maybe @nlutsenko can quickly answer here? |
have also #39 fix proposed. |
Installation ID is passed as a header to every single API request sent against a server. |
} | ||
|
||
router.route('POST', '/functions/:functionName', handleCloudFunction); | ||
router.route('POST', '/functions/:functionName+', handleCloudFunction); |
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.
Why the + sign here?
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.
- will allow to have a structured calls on functions, such as
Parse.Cloud.run('workflow/list', { objs : true}),
extending functions reach.
It is an optional route parameter from express, it will be ignored if it is not present and it will work as pervious.
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.
It requires latest express and path-to-regexp version, otherwise it will fallback to initial behaviour.
The installation id is parsed from the headers here: https://github.com/ParsePlatform/parse-server/blob/master/middlewares.js#L22 This is a reasonable solution for the client class creation option... I wanted it to be an option passed to ParseServer, but this is easier and makes sense. Anyone have an issue with it? |
I think it's better to provide an option that you can pass to ParseServer, so you have more control (e.g. sometimes I don't want schema to update in dev env or sometimes I do want it to update on production). Maybe that should be in a separate PR too? |
A more complex solution will be needed if/when ParseServer will be multi-application, and this may be an architectural decision. This is the reason why I did preferred to get up-and-running, solution. @andrewoons : Env variable may be easily changed outside of code, and if you send masterKey you may be able to change everything. May be used a distinct env variable name to configure, but I don't think it will make a real difference. |
@lucianmat any specific reason made you close this PR? |
Yeah I'd still like to see the freeze method here for disabling client-class-creation... I understand if you were just cleaning this up because of all the rebasing.. Also, the req.user issue with functions got updated already in #172. |
Awesome! |
@natanrolnik it needed to rebase the code, as @gfosco had guessed . I'll resubmit PR after cleaning up, and adding some tests and documentation for proposed behaviour. |
added user, isMaster, installationId on functions as Parse.Cloud.FunctionRequest specs .
changed route path to '/functions/:functionName+', that way function can be requested with extra path components as : Parse.Cloud.run('workflows/list', {})