Conversation
WalkthroughThis pull request introduces a new auto-review configuration in the Changes
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 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
Documentation and Community
|
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
✨ Benchmark results
⚡ Benchmark Comparison
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.coderabbit.yaml (1)
1-7: Missing newline at end of file.The YAML configuration looks good, but there is a linting issue detected. YAMLlint reports that there's no newline character at the end of the file.
Add a newline character at the end of the file to fix this linting issue:
- 1.6.x - 1.7.x - feat-sites # temporary until merged to 1.7.x +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 7-7: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.coderabbit.yaml(1 hunks)app/config/collections/projects.php(2 hunks)src/Appwrite/Platform/Modules/Functions/Http/Deployments/Create.php(2 hunks)src/Appwrite/Platform/Modules/Functions/Http/Deployments/Duplicate/Create.php(1 hunks)src/Appwrite/Platform/Modules/Functions/Workers/Builds.php(3 hunks)src/Appwrite/Platform/Modules/Sites/Http/Deployments/Create.php(2 hunks)src/Appwrite/Platform/Modules/Sites/Http/Deployments/Duplicate/Create.php(1 hunks)src/Appwrite/Utopia/Database/Validator/Queries/Deployments.php(1 hunks)src/Appwrite/Utopia/Response/Model/Deployment.php(1 hunks)tests/e2e/Services/Functions/FunctionsCustomServerTest.php(5 hunks)tests/e2e/Services/Sites/SitesCustomServerTest.php(4 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.coderabbit.yaml
[error] 7-7: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (VCS)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Projects)
- GitHub Check: E2E Service Test (Users)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: Benchmark
- GitHub Check: E2E General Test
🔇 Additional comments (25)
src/Appwrite/Utopia/Database/Validator/Queries/Deployments.php (1)
10-10: Good addition oftotalSizeto the allowed attributes.This change correctly adds the new attribute to the
ALLOWED_ATTRIBUTESarray, which will enable querying deployments based on this value. This supports the PR objective of enhancing integration with the Console and providing query capabilities.src/Appwrite/Platform/Modules/Sites/Http/Deployments/Create.php (2)
246-246: Successfully addedtotalSizeattribute to deployment record.The new
totalSizeattribute is correctly initialized with the$fileSizevalue, which matches the current behavior ofsourceSize. This change supports the PR's objective of introducing the attribute that represents the total deployment size.
304-304: Consistently implementedtotalSizeattribute for chunked uploads.The
totalSizeattribute is correctly added to the chunked upload case, maintaining consistency with the non-chunked implementation.src/Appwrite/Platform/Modules/Functions/Http/Deployments/Duplicate/Create.php (1)
103-103: Correctly propagatedtotalSizeattribute during deployment duplication.The new
totalSizeattribute is successfully added to the duplicate deployment, using the source deployment'ssourceSizevalue with a fallback to 0 if not present.src/Appwrite/Platform/Modules/Functions/Http/Deployments/Create.php (2)
237-237: Good implementation of the newtotalSizeattribute.The addition of
totalSizeattribute is correctly implemented as part of the deployment creation process, initializing it with the value of$fileSize. This matches the PR's objective of introducing atotalSizeattribute to enhance integration with the Console.
268-268: Consistent implementation oftotalSizein chunked upload scenario.The
totalSizeattribute is correctly initialized for chunked uploads as well, ensuring consistency in how deployment size is tracked regardless of the upload method.src/Appwrite/Utopia/Response/Model/Deployment.php (1)
67-72: Well-defined model rule fortotalSizeattribute.The new rule for
totalSizeis properly defined with the correct type (integer), an appropriate description that clearly explains it represents the sum of source and build output sizes, and sensible default/example values. This addition aligns with the PR objectives and provides the necessary model definition for the new attribute.src/Appwrite/Platform/Modules/Sites/Http/Deployments/Duplicate/Create.php (2)
114-114: Good implementation oftotalSizein deployment duplication.The
totalSizeattribute is correctly initialized during deployment duplication, using the original deployment'ssourceSizevalue. This ensures the new attribute is properly propagated when duplicating deployments.
122-123: Improved initialization of numeric attributes.Changing the initialization of
buildDurationandbuildSizefromnullto explicit0values improves type consistency for these numeric fields, which is a good practice.app/config/collections/projects.php (2)
1553-1563: Proper schema definition fortotalSizeattribute.The schema definition for the new
totalSizeattribute is correctly implemented with the appropriate type (integer), along with necessary metadata for the database schema. This aligns with the model definition in the Response class.
1634-1640: Good addition of index for query performance.Adding an index for the
totalSizeattribute is a good practice as it will improve query performance when filtering or sorting by this field. The index is properly configured to sort in ascending order, consistent with the other size-related indexes.src/Appwrite/Platform/Modules/Functions/Workers/Builds.php (3)
288-289: LGTM: Properly initializingtotalSizeattributeInitial setting of
totalSizeequal tosourceSizeduring the first stage of deployment makes sense since build hasn't been completed yet.
440-441: LGTM: Consistent approach to initializingtotalSizeThis mirrors the same pattern used earlier in the file, maintaining consistency in how
totalSizeis initially set.
725-725: LGTM: Correctly calculating finaltotalSizevalueThis properly updates the
totalSizeattribute to be the sum ofbuildSizeandsourceSizeafter the build is complete, which aligns with the feature's intent.tests/e2e/Services/Functions/FunctionsCustomServerTest.php (5)
412-415: LGTM: Good test coverage for initial deployment stateThese assertions correctly verify that
sourceSize,buildSize, andtotalSizeare all initially zero before the deployment is processed.
489-494: LGTM: Comprehensive verification oftotalSizecalculationGood test assertions that verify both source and build sizes are greater than zero after deployment, and that
totalSizeequals their sum.
1673-1678: LGTM: Consistent test coverage in cookie execution testThese assertions maintain consistency with other tests, verifying the same properties in a different test scenario.
2061-2065: LGTM: Test coverage for duplicate deployment scenarioImportant test case that verifies the initial state of a duplicate deployment where
buildSizeis 0 butsourceSizeis already copied, ensuringtotalSizeequals just thesourceSizeat this stage.
2076-2081: LGTM: Complete test coverage for post-execution stateThese assertions correctly verify that after execution, both source and build sizes are populated and
totalSizeproperly equals their sum.tests/e2e/Services/Sites/SitesCustomServerTest.php (6)
363-369: Great test coverage for the new totalSize attributeThe assertions correctly verify that the totalSize is the sum of sourceSize and buildSize after a successful deployment response. This is a good implementation of the new feature requirement.
1520-1525: Good validation of initial deployment stateThese assertions properly verify that when a template deployment is initially created, all size attributes (sourceSize, buildSize, totalSize) are set to 0, which makes sense since no resources have been allocated yet.
1547-1553: Good validation of deployment after completionThis test section correctly verifies that after the deployment completes, the totalSize reflects the sum of sourceSize and buildSize. This ensures the integrity of the size calculations throughout the deployment lifecycle.
2003-2014: Good implementation of duplicate deployment checkThe test correctly verifies that for a newly duplicated deployment, the totalSize initially equals the sourceSize since buildSize is 0 at creation time. This is a good edge case to test.
2015-2019: Well-structured wait for deployment activationGood implementation of assertEventually to wait for the deployment to be activated and properly set as the site's active deployment.
2023-2029: Complete verification of final deployment stateThis section thoroughly validates that after the deployment is fully processed, the totalSize correctly equals the sum of sourceSize and buildSize. This ensures the size calculations are maintained properly throughout the deployment lifecycle.
What does this PR do?
Adds totalSize to deployment, sum of soruceSize and outputSize. Storing this in DB allows easy integration and consistency with Console, plus ability to query by this total.
Test Plan
Tests updated
Related PRs and Issues
x
Checklist
Summary by CodeRabbit
New Features
Tests