-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Reorganization of ParseServerOptions #7069
Comments
Thanks for suggesting. This would certainly be an improvement in terms of configuration structure and oversight. However, as discussed in the past, this would have to be look at holistically as it would be a breaking change affecting virtually every Parse Server deployment. The change efforts required can vary greatly depending on the specific environment, automation complexity and setup in which Parse Server is deployed. Internally, changed would have to be made in:
Ergo, the frequency of such changes should be kept as low as possible and any fundamental overhaul of the Parse Server configuration should be well crafted for the foreseeable future. Such a change may as well justify - or be part of - a Parse Server major version increment. All this goes to say that just regrouping parameters may not be all that should be considered here, but also:
|
Fair comments. It’s my view that the problem will continue to get worse, however I agree it’s hard to justify such a breaking change for such a nit. Personally I think cacheOptions, GraphQLoptions and keys are probably the only ones that need better organisation. |
I think it justifiable, just needs to be looked at holistically. I think it should be taken on. I would advice to first discuss the intend changed here in detail before creating a PR because this will require so many changes that the PR will likely need several rebases/merges and face merge conflicts. I suggest this process:
|
i think the Parse Server Options need this kind of re work, and group of options proposed by @dblythy at first glance seems not bad at all. But the breaking change is massive for parse developers and also for all of our internal code (tests, etc...) During one major version we need to support current options and the new one. Also since changes could be huge and lead to massive PR. I would propose also a development process:
Do it again for each option group. Finally when What do you think about that ? |
I'm supportive of slowly phasing out the old options while still supporting them alongside with the new options, if that does not complicate the code too much or introduce other costs. This change has no functional implication, it is merely aesthetics, so the costs and risks for however this is done should be kept as low as possible. Another strategy to consider may be introducing a config version parameter as part of the configuration. The parameter can default to the old config, but developers can set it to the new config and use the new structure. Internally, this only requires a translation method that translates the old config into the new config. It does not require multiple PRs or to change all tests at once. It is similar to AWS policy versioning but for Parse Server configuration:
We may benefit from this feature in the future, so the efforts are not for just a one time change. |
So with the config version idea, Config.validate would translate according to the configVersion? e.g, configVersion 20210101 converts |
Yes, along these lines, but that's just an idea, maybe that complexity is not needed after all. |
I would imagine in terms of lines changed it would be the simplist change (rather than changing every single mention of I'd imagine ENV variables would have to stay the same as they are, which I don't think will be too big of a deal. |
Well, that's something we'd have to figure out. It could create more confusion if there is a discrepancy between the configuration structure on how to access the config internally. I think internally, the config should be adapted to the new structure, because it is more developer friendly, and the old config should be transposed to the internal new config. But again, this approach was just an idea, the effort may not be necessary. |
Just noticed, I don't think |
@mtrezza After working with https://www.npmjs.com/package/convict i think it could be a really good way to expose a configuration interface as it allow to define a configuration schema, and validation. It also allow binding to ENV, and defining |
Hi @mtrezza sorry for the delay, I'm trying to catch up on stuff that is accumulating ! One thing I like with Convict is that it does all the co fig validation, and handle multiple types of configs (yaml, JSON, ENV Vars....) The format is also intuitive to use. Currently, my guess is thatmost new projects start by cloning the I think all the configuration should be kept out of the main script into a separate file. This would allow a few things:
For example currently bellow the
|
Any improvements to the |
How do you see this being used in the example repo? |
@mtrezza I'm using it in a project, you can have a look here at: |
2 thoughts:
|
You are absolutely right. @mtrezza Convict would be more for My thought on this is that your example project should be as easy and convenient to get started with. Adding a bit of complexity is fairly OK if it allow users can simply take the example project to start their own projects. |
I think convict could be a valuable addition to Parse Server though. @sadortum, can you tell which features of Parse Server's current config mechanism convict already covers? |
Is your feature request related to a problem? Please describe.
As Parse Server continues to evolve, so do the Parse Server options.
Describe the solution you'd like
There are certain options that in my view should be grouped together for neater organization, such as:
User Options:
-accountLockout
-enableAnonymousUsers
-expireInactiveSessions
-passwordPolicy
-preventLoginWithUnverifiedEmail
-revokeSessionOnPasswordReset
-sessionLength
-userSensitiveFields
Database Options:
-allowClientClassCreation
-allowCustomObjectId
-collectionPrefix
-databaseOptions
-objectIdSize
-protectedFields
REST Options:
-allowHeaders
-allowOrigin
-enableExpressErrorHandler
-middleware
Adapters:
-analyticsAdapter
-auth
-cacheAdapter
-databaseAdapter
-emailAdapter
-filesAdapter
-loggerAdapter
-push
Keys:
-clientKey
-databaseURI
-dotNetKey
-encryptionKey
-fileKey
-javascriptKey
-masterKey
-readOnlyMasterKey
-restAPIKey
-webhookKey
Server Info:
-appId
-appName
-cloud
-cluster
-customPages
-directAccess
-host
-jsonLogs
-masterKeyIps
-maxLimit
-mountPath
-port
-publicServerURL
-serverURL
Cache:
-cacheMaxSize
-cacheTTL
-enableSingleSchemaCache
-schemaCacheTTL
Email Options:
-emailVerifyTokenReuseIfValid
-emailVerifyTokenValidityDuration
-verifyUserEmails
Graph QL:
-graphQLPath
-graphQLSchema
-mountGraphQL
-mountPlayground
-playgroundPath
Idempotency:
-idempotencyOptions
Live Query:
-liveQuery
-liveQueryServerOptions
-startLiveQueryServer
Logs:
-logLevel
-logsFolder
-maxLogFiles
-silent
-verbose
Files:
-maxUploadSize
-preserveFileName
Push:
-scheduledPush
Server Events:
-serverCloseComplete
-serverStartComplete
I don't think every option requires categorization, but I think it could be handy to have all similar options tied together and cleaner in the docs, such as:
Considerations:
-How to handle options that could be under 2 categories, such as emailAdapter, which could be under adapters and email options
-This change would either need to be backwards compatible, or be a breaking change requiring changes to existing index.js.
The goal of this would be to ensure consistency and maintainability for ParseServerOptions, and to prevent them from continuing to bloat.
Please let me know your thoughts.
The text was updated successfully, but these errors were encountered: