feat: Postgres base as schema #9591
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request includes significant updates to the environment variable documentation and related code functionalities. A new environment variable, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (8)
packages/nocodb/src/providers/init-meta-service.provider.ts (1)
Line range hint
1-122: LGTM: Initialization process structure maintainedThe overall structure and flow of the initialization process have been maintained, with the only change being the repositioning of
initBaseBehavior. This change appears intentional and likely aims to ensure that the base behavior is set up after all other components are initialized.Consider updating the comments at the beginning of the
useFactoryfunction to reflect the new order of operations, including theinitBaseBehaviorstep at the end. This will help maintain clear documentation of the initialization process.packages/noco-docs/docs/020.getting-started/050.self-hosted/020.environment-variables.md (4)
78-96: LGTM! Consider adding a link to the Postgres Base as Schema section.The changes in the "Product Configuration" section look good. The new variable
NC_DISABLE_BASE_AS_PG_SCHEMAis well-described, and the updates to existing variable descriptions improve clarity.Consider adding a direct link to the "Postgres Base as Schema" section in the description of
NC_DISABLE_BASE_AS_PG_SCHEMAfor easier navigation. For example:-| `NC_DISABLE_BASE_AS_PG_SCHEMA` | No | Disables the creation of a schema for each base in PostgreSQL. [Click here for more detail](#postgres-base-as-schema) | | +| `NC_DISABLE_BASE_AS_PG_SCHEMA` | No | Disables the creation of a schema for each base in PostgreSQL. See [Postgres Base as Schema](#postgres-base-as-schema) for more details. | |🧰 Tools
🪛 LanguageTool
[uncategorized] ~87-~87: The official spelling of this programming framework is “Express.js”.
Context: ... allowed in the request body, based on [ExpressJS limits](https://expressjs.com/en/resour...(NODE_JS)
97-99: Expand the explanation of the Postgres Base as Schema feature.The new section provides a good introduction to the Postgres Base as Schema feature. However, it could benefit from a slightly more detailed explanation.
Consider expanding the explanation to include:
- The benefits of using this feature (e.g., improved organization, security, or performance).
- Any potential implications of disabling it.
- A brief example of how it affects database structure.
Here's a suggested expansion:
### Postgres Base as Schema For PostgreSQL, a unique schema is created for each base, providing logical separation within the database. This feature offers several benefits: - Improved organization: Each base's tables are grouped in their own schema. - Enhanced security: It's easier to manage permissions at the schema level. - Reduced naming conflicts: Table names can be reused across different bases. This feature is enabled by default if the user has the required permissions. To disable it, set the `NC_DISABLE_BASE_AS_PG_SCHEMA` environment variable to `true`. Note that disabling this feature may affect how your database is organized and could impact existing queries or permissions if you've been relying on the schema structure. Example: With this feature enabled, a base named "Project A" might create its tables in a schema called "project_a" instead of the public schema.
Line range hint
100-134: LGTM! Consider adding a brief introduction to Litestream.The new Litestream section is comprehensive and well-structured. It provides clear descriptions for all the new environment variables and includes helpful deprecation notices.
To improve clarity for users who might not be familiar with Litestream, consider adding a brief introduction at the beginning of the section. For example:
## Litestream Litestream is a tool that provides replication and disaster recovery for SQLite databases. In NocoDB, it's used to back up the SQLite database to S3-compatible storage when `NC_DB` is set to SQLite. > Note: Litestream is used **only** when `NC_DB` is set to SQLite. It backs up the SQLite database and stores it in S3-compatible storage. | Variable | Mandatory | Description | If Not Set | ...This introduction provides context for the subsequent configuration options and clarifies when Litestream is relevant to NocoDB users.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~87-~87: The official spelling of this programming framework is “Express.js”.
Context: ... allowed in the request body, based on [ExpressJS limits](https://expressjs.com/en/resour...(NODE_JS)
87-87: Minor: Update "ExpressJS" to "Express.js" for consistency.For consistency with official naming conventions, consider updating "ExpressJS" to "Express.js".
Apply this change:
-| `NC_REQUEST_BODY_SIZE` | No | Maximum bytes allowed in the request body, based on [ExpressJS limits](https://expressjs.com/en/resources/middleware/body-parser.html#limit). | Defaults to `1048576` (1 MB). | +| `NC_REQUEST_BODY_SIZE` | No | Maximum bytes allowed in the request body, based on [Express.js limits](https://expressjs.com/en/resources/middleware/body-parser.html#limit). | Defaults to `1048576` (1 MB). |🧰 Tools
🪛 LanguageTool
[uncategorized] ~87-~87: The official spelling of this programming framework is “Express.js”.
Context: ... allowed in the request body, based on [ExpressJS limits](https://expressjs.com/en/resour...(NODE_JS)
packages/nocodb/src/helpers/initBaseBehaviour.ts (1)
68-78: Avoid modifyingprocess.envat runtime for configuration flags.Modifying environment variables at runtime can lead to unexpected behavior, especially if other modules read these variables at startup. It's better to use a configuration object or a dedicated configuration management solution to manage feature flags within the application.
packages/nocodb/src/services/bases.service.ts (2)
190-195: Moverequirestatements to the top of the fileFor better code organization and readability, it's a good practice to place all
requireorimportstatements at the beginning of the file.Please move the following declarations to the top:
+ const fs = require('fs').promises; ... + const nanoidv2 = customAlphabet('1234567890abcdefghijklmnopqrstuvwxyz', 14);This helps maintain consistency and makes dependencies easier to locate.
Line range hint
222-231: Avoid code duplication by refactoring shared logicThe code for setting
baseBody.sourceshas similar structures in different conditional branches. Refactoring can improve maintainability.Consider extracting the common parts or creating helper functions to handle the source configuration setup. This reduces repetition and potential errors in future modifications.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- packages/noco-docs/docs/020.getting-started/050.self-hosted/020.environment-variables.md (1 hunks)
- packages/nocodb/src/helpers/initBaseBehaviour.ts (2 hunks)
- packages/nocodb/src/providers/init-meta-service.provider.ts (1 hunks)
- packages/nocodb/src/services/bases.service.ts (1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/noco-docs/docs/020.getting-started/050.self-hosted/020.environment-variables.md
[uncategorized] ~87-~87: The official spelling of this programming framework is “Express.js”.
Context: ... allowed in the request body, based on [ExpressJS limits](https://expressjs.com/en/resour...(NODE_JS)
🔇 Additional comments (3)
packages/nocodb/src/providers/init-meta-service.provider.ts (1)
116-118: Verify the impact of repositioninginitBaseBehaviorThe
initBaseBehaviorfunction call has been moved to the end of the initialization process. While this change ensures that all other components are initialized before setting up the base behavior, it's important to verify that this repositioning doesn't have any unintended consequences.Consider the following:
- Does the base behavior depend on any components that were previously initialized after it?
- Are there any components or processes that depend on the base behavior being initialized earlier in the process?
To ensure this change doesn't introduce any issues, please run the following verification:
Please review the results of this verification to ensure that the repositioning of
initBaseBehaviordoesn't break any existing functionality or dependencies.✅ Verification successful
Repositioning
initBaseBehaviorVerifiedThe movement of the
initBaseBehaviorfunction call to the end of the initialization process has been confirmed not to affect other components or dependencies within the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any dependencies on initBaseBehavior throughout the codebase # Search for direct calls or references to initBaseBehavior echo "Searching for direct calls or references to initBaseBehavior:" rg "initBaseBehavior" --type ts # Search for any code that might depend on the base behavior being set echo "\nSearching for potential dependencies on base behavior:" rg "getBaseBehavior|setBaseBehavior" --type ts # Check if initBaseBehavior is exported and used elsewhere echo "\nChecking if initBaseBehavior is exported and used elsewhere:" rg "export.*initBaseBehavior" --type ts rg "import.*initBaseBehavior" --type tsLength of output: 1173
packages/noco-docs/docs/020.getting-started/050.self-hosted/020.environment-variables.md (1)
Line range hint
1-134: Overall, excellent updates to the environment variables documentation.The changes in this file significantly improve the NocoDB environment variables documentation. The addition of the Litestream section, the new Postgres Base as Schema feature explanation, and the updates to existing variable descriptions enhance the overall clarity and completeness of the document.
The suggested improvements, if implemented, will further refine the document:
- Adding a link to the Postgres Base as Schema section for easier navigation.
- Expanding the explanation of the Postgres Base as Schema feature.
- Including a brief introduction to Litestream.
- Updating "ExpressJS" to "Express.js" for consistency.
These changes will make the documentation more user-friendly and informative for NocoDB users.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~87-~87: The official spelling of this programming framework is “Express.js”.
Context: ... allowed in the request body, based on [ExpressJS limits](https://expressjs.com/en/resour...(NODE_JS)
packages/nocodb/src/services/bases.service.ts (1)
169-170: Ensure consistent handling of environment variablesThe environment variables
NC_DISABLE_BASE_AS_PG_SCHEMAandNC_MINIMAL_DBSare being compared to the string'true'. This approach can lead to unexpected behavior if the environment variable is not set or has different casing.Consider normalizing and parsing environment variables as booleans:
const isPgSchemaEnabled = process.env.NC_DISABLE_BASE_AS_PG_SCHEMA !== 'true'; ... if ( dataConfig?.client === 'pg' && - process.env.NC_DISABLE_BASE_AS_PG_SCHEMA !== 'true' + isPgSchemaEnabled ) {Similarly for
NC_MINIMAL_DBS:const isMinimalDbsEnabled = process.env.NC_MINIMAL_DBS === 'true'; ... if ( dataConfig?.client === 'sqlite3' && - process.env.NC_MINIMAL_DBS === 'true' + isMinimalDbsEnabled ) {This clarifies the intent and reduces the chance of errors due to string comparisons.
Also applies to: 186-187
Signed-off-by: Pranav C <pranavxc@gmail.com>
Change Summary
Provide summary of changes with issue number if any.
Change type
Test/ Verification
Provide summary of changes.
Additional information / screenshots (optional)
Anything for maintainers to be made aware of