appsmithorg
chore: Refactor DB changelog to install plugins in workspace with single query
Description
The method currently makes multiple DB calls
- Fetch all workspace ids where the plugin id is not present
- Fetch complete workspace details with these ids (1 query per workspace id)
- Assign plugin and save each workspace
Fixes #16762
The fix performs one single update query. The query adds the plugin to all workspaces where the plugin doesn't already exist. No data is fetched in java
Type of change
- Breaking change (fix or feature that would cause existing functionality to not work as expected)
How Has This Been Tested?
No tests found in the repo. Check if installation of plugin to all workspaces works when there are multiple workspaces and also when there are none.
- When there are no workspaces where the plugin is not installed
- There are multiple workspaces where the plugin is not installed
Checklist:
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] New and existing unit tests pass locally with my changes
@bhuvanaindukuri is attempting to deploy a commit to the Appsmith Team on Vercel.
A member of the Team first needs to authorize it.
![vercel[bot] avatar](https://avatars.githubusercontent.com/in/8329?v=4 "Published by a pub.dev verified publisher"){kind=small}
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Updated |
|---|---|---|---|
| appsmith | ✅ Ready (Inspect) | Visit Preview | Oct 28, 2022 at 11:58AM (UTC) |
@bhuvanaindukuri this does not look right. The goal of this issue is that we should not be fetching any data in the application memory. Instead we want to create a DB query (just one query) that simply runs on the DB and does the job - which means that we should not be requiring any result from the query.
@bhuvanaindukuri this does not look right. The goal of this issue is that we should not be fetching any data in the application memory. Instead we want to create a DB query (just one query) that simply runs on the DB and does the job - which means that we should not be requiring any result from the query.
@sumitsum - Updated the code as per the requirement
@bhuvanaindukuri can you please explain in the description what your change does ?
@bhuvanaindukuri can you please explain in the description what your change does ?
Updated the details of the fix in the PR description
@sumitsum - hope this is what you had requested. Please let me know if not
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@bhuvanaindukuri the PR does not exactly do what was intended but does it partially. The bulk update operation seems to perform the updates in batches instead of all at once - which is still much better than doing it one by one as it was being done before. Can you please create a new GitHub issue mentioning the same and link this PR against it ? I am approving the PR. Ref: https://stackoverflow.com/questions/53296516/is-there-a-default-batch-size-that-is-used-by-mongodb-in-the-bulk-api Ref: https://www.mongodb.com/community/forums/t/performance-bottleneck-in-bulk-insertion-java/8092/5
@bhuvanaindukuri can you please merge with latest release and update the PR so that CI run may have a better chance at passing.
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3340014606.
Workflow: Appsmith External Integration Test Workflow.
Commit: cd8f2cf.
PR: 17395.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=17395&runId=3340014606_1
@bhuvanaindukuri this may be helpful when updating the branch with latest release: https://stackoverflow.com/questions/7244321/how-do-i-update-or-sync-a-forked-repository-on-github Also, would it be possible for you to provide my account with push / edit access to your PR branch so that I may do such kind of updates(rebase) if required (in case you are not accessible) ?
@bhuvanaindukuri seems like I have write access to the PR. I have rebased the PR. I will now run CI.
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3345987533.
Workflow: Appsmith External Integration Test Workflow.
Commit: 3d85418.
PR: 17395.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=17395&runId=3345987533_1
Adding test plan approved label since this is a database migration related change. cc: @prapullac