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

Support direct access server option #5550

Merged
merged 9 commits into from
May 10, 2019
Merged

Support direct access server option #5550

merged 9 commits into from
May 10, 2019

Conversation

dplewis
Copy link
Member

@dplewis dplewis commented May 1, 2019

Closes: #5442

Let me know if I missed anything or wording needs clarifying. I've never added a server config option before.

@dplewis dplewis requested a review from acinader May 1, 2019 14:29
@omairvaiyani
Copy link
Contributor

I'd love to quantify how much time this will save per request, I suspect it'll be quite noticeable at scale. Nice work.

README.md Outdated
@@ -245,6 +245,7 @@ The client keys used with Parse are no longer necessary with Parse Server. If yo
* `auth` - Used to configure support for [3rd party authentication](http://docs.parseplatform.org/parse-server/guide/#oauth-and-3rd-party-authentication).
* `facebookAppIds` - An array of valid Facebook application IDs that users may authenticate with.
* `mountPath` - Mount path for the server. Defaults to `/parse`.
* `directAccess` - Replace HTTP Interface when using JS SDK in current node runtime. Defaults to false.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe added something like: 'Caution, this is an experimental feature that may not be appropriate for production'

Copy link
Member Author

Choose a reason for hiding this comment

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

We should remove this section, organize the definition in alphabetical order and replace with http://parseplatform.org/parse-server/api/3.3.0/ParseServerOptions.html

@@ -50,6 +51,7 @@
* @property {Number} schemaCacheTTL The TTL for caching the schema for optimizing read/write operations. You should put a long TTL when your DB is in production. default to 5000; set 0 to disable.
* @property {Number} cacheTTL Sets the TTL for the in memory cache (in ms), defaults to 5000 (5 seconds)
* @property {Number} cacheMaxSize Sets the maximum size for the in memory cache, defaults to 10000
* @property {Boolean} directAccess Replace HTTP Interface when using JS SDK in current node runtime
Copy link
Contributor

Choose a reason for hiding this comment

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

add default false. also should point out that it is experimental

Copy link
Member Author

Choose a reason for hiding this comment

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

I realized updating Parse Server config definitions isn't documented and isn't run on every PR. I can submit a separate PR for that

@acinader
Copy link
Contributor

acinader commented May 4, 2019

@omairvaiyani the reason that it hasn't been turned on is because the unit tests fail.

@dplewis you could add this to the travis matrix and just make it optional so we can at least see what we're up against?

@codecov
Copy link

codecov bot commented May 8, 2019

Codecov Report

Merging #5550 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5550      +/-   ##
==========================================
- Coverage   94.01%   93.99%   -0.03%     
==========================================
  Files         129      129              
  Lines        9192     9192              
==========================================
- Hits         8642     8640       -2     
- Misses        550      552       +2
Impacted Files Coverage Δ
src/Options/index.js 100% <ø> (ø) ⬆️
src/Options/Definitions.js 100% <ø> (ø) ⬆️
src/ParseServer.js 93.43% <100%> (-2.92%) ⬇️
src/RestWrite.js 93.71% <0%> (+0.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81ecf2f...628cadf. Read the comment docs.

@dplewis
Copy link
Member Author

dplewis commented May 9, 2019

@acinader I reverted travis.yml because the matrix stakes too long. At least we can see what some of the problems are. Looks like there are issues with CLP, ACL, MasterKey and SessionToken.

https://travis-ci.org/parse-community/parse-server/jobs/529921508

Besides that, how does this look?

Copy link
Contributor

@acinader acinader left a comment

Choose a reason for hiding this comment

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

it looks good to me.

see my question.

action: parsers.objectParser,
default: { _User: { '*': ['email'] } },
default: [],
Copy link
Contributor

Choose a reason for hiding this comment

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

why the change to the default?

Copy link
Member Author

@dplewis dplewis May 9, 2019

Choose a reason for hiding this comment

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

This is autogenerated. Parser doesn't exist for this json. I can fix this in a separate PR.

@dplewis dplewis merged commit b4d915b into master May 10, 2019
@dplewis dplewis deleted the direct-access branch May 10, 2019 19:34
UnderratedDev pushed a commit to UnderratedDev/parse-server that referenced this pull request Mar 21, 2020
* Support direct access config

test options

* add test

* fix test

* fix definitions

* improve docs

* Update .travis.yml

* Revert "Update .travis.yml"

This reverts commit 407f138.
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.

Questions about Direct Access Feature
3 participants