Skip to content
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

Added ability to set server url for Parse JS client SDK #132

Merged
merged 1 commit into from
Feb 4, 2016
Merged

Added ability to set server url for Parse JS client SDK #132

merged 1 commit into from
Feb 4, 2016

Conversation

r4zzz4k
Copy link
Contributor

@r4zzz4k r4zzz4k commented Feb 1, 2016

Here is the reason for this change: #131
With this fix we can simply add new argument to ParseServer constructor:

var api = new ParseServer({
  databaseURI: '...',
  cloud: '...',
  appId: '...',
  ...
  serverUrl: 'http://localhost:' + port
});

After this piece of code JS client SDK will start communicating with local instance of Parse Server and not with parse.com.

@r4zzz4k
Copy link
Contributor Author

r4zzz4k commented Feb 1, 2016

Actually I'm sure there is a way to do everything better, but I don't have enough experience with Node.js to suggest anything better. So I'm open to discussion.

@gfosco
Copy link
Contributor

gfosco commented Feb 1, 2016

This is definitely valuable... just not sure the best way to do it. It does seem strange/unfortunate to have to tell the server it's own URL.

@r4zzz4k
Copy link
Contributor Author

r4zzz4k commented Feb 1, 2016

Well, another option will be to change JS SDK so that it can understand that it's running alongside Parse Server and get somehow protocol (I mean http / https) and port it's running on or even change the way they communicate, but passing URL from Parse Server to Parse JS Client seems like reasonable idea, at least until deeper changes will be made.
On the other hand I'm sure that it's possible for Parse Server initialize JS SDK in more meaningful way without hand-crafting server url by programmer, but I know nearly nothing about how to work with express so I can't do anything better currently.

@r4zzz4k
Copy link
Contributor Author

r4zzz4k commented Feb 1, 2016

Okay, I've just looked into JS SDK and made the change a bit easier to read.

@gfosco
Copy link
Contributor

gfosco commented Feb 1, 2016

Ok, please update the README adding this option and updating the example.

@r4zzz4k
Copy link
Contributor Author

r4zzz4k commented Feb 1, 2016

Done. Hope that prettier solution will be found in the future :)

@gfosco
Copy link
Contributor

gfosco commented Feb 2, 2016

I think we should stick with serverURL instead of cloudCodeServerURL

@gfosco gfosco mentioned this pull request Feb 2, 2016
@r4zzz4k
Copy link
Contributor Author

r4zzz4k commented Feb 2, 2016

Fixed. I thought for a bit that it's possible to make Cloud Code actually use different URL so I've decided to specify it explicitly but actually it's quite unrealistic case. So agreed.

@steven-supersolid
Copy link
Contributor

Is there a ticket open to change how Cloud Code communicates with Parse Server?

Maybe I'm wrong but wouldn't a Cloud Code query operation send a http request to Parse Server which will be running on the same appserver cluster to perform the query? This seems inefficient as two processes (or Node equivalent) will be involved plus there is the overhead of the http connection for what could just be a function call.

@dadicool
Copy link

dadicool commented Feb 2, 2016

This is definitely valuable... just not sure the best way to do it. It does seem strange/unfortunate to have to tell the server it's own URL.

In cases where parse server is deployed behind a load balancer for example, the ability to specify its "public" URL comes in handy, and there is no way the server itself can figure it out without this setting. /cc @gfosco

@@ -52,7 +55,8 @@ var api = new ParseServer({
cloud: '/home/myApp/cloud/main.js', // Provide an absolute path
appId: 'myAppId',
masterKey: 'mySecretMasterKey',
fileKey: 'optionalFileKey'
fileKey: 'optionalFileKey',
serverURL: 'http://localhost:' + port // Don't forget to change to https if needed
Copy link
Contributor

Choose a reason for hiding this comment

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

this would also need the /parse or whatever the mount point is. For this readme, I'd include a full example url, like http://localhost:1337/parse

@jamiechapman
Copy link

I was wondering about this myself, it does seem strange to tell the server its own URL. But eventually for things like password resets, it wouldn't be nice to be able to point toward a custom domain etc. Maybe if a serverURL isn't defined explicitly (or in the environment settings) it defaults to os.hostname()?

@lucianmat
Copy link

Wouldn't make sense to have a direct link to server API from parse/node library, using Providers linked directly to rest controllers ? Direct call it will save an extra http call, with a better speed and less resources.

Someting like linking ObjectController to rest.find by wrapping ... ?

@gfosco
Copy link
Contributor

gfosco commented Feb 2, 2016

@lucianmat absolutely that would be nice, it's just a separate issue. Would love to see that happen.

@jamiechapman Is there a precedent for node services to use os.hostname() for determining its own address?

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.

6 participants