Batch upload & other performance enhancements #674
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A few places make Django ORM queries at the module level and thus run during import of the portal code. This implies that a working connection to a properly set-up database is required to e.g. use the Django management command. This greatly complicates packaging, initial setup, and deployment. This problem is fixed by simple refactorings and caches.
Creation of the SQLAlchemy model caches occurs during import of the portal code. This implies that a working connection to a properly set-up database is required to e.g. use the Django management command. This greatly complicates packaging, initial setup, and deployment. This problem is fixed by ignoring errors creating the cache and reminding the user to check the database.
The `iterkeys()` method on a dict has been replaced by the equivalent `keys()` method in Python 3. Update to use the correct method.
Reject duplicate names in the controlled vocabularies list. It is unknown why such duplicates are being served, but they are as of 2022-12-27.
352KB file (one column): 148s -> 1.6s
subrepo: subdir: "ODM2DataSharingPortal" merged: "33b2ff2f" upstream: origin: "https://github.com/ODM2/ODM2DataSharingPortal" branch: "main" commit: "e3dd03c2" git-subrepo: version: "0.4.1" origin: "???" commit: "???"
…nsor measurements
|
Thanks again for the discussion and allocating time for this. Please note that there are several commits in here that are for my working process and should be dropped before merging. This branch was not intended to be merged directly. If I can get the site working locally with the Cognito integration then I can rebase but otherwise it is probably best to cherry-pick or have @ptomasula implement the changes in spirit rather than merge directly. |
This was referenced Sep 20, 2023
aufdenkampe
added this to the v0.18 release. EnviroDIY performance improvements milestone
|
@aufdenkampe , @ScottEnsign @SRGDamia1 As @tpwrules noted, the tpwrules:wip/batch branch (current set for this PR) is a work in progress branch and not intended to merge in directly. I followed Thomas's recommendation and cherry-picked out select commits (see table below) into a new batch_upload. I'll be updating this PR to use this new batch_upload branch and work on testing and closing things out on there.
|
|
Excited to see the progress! Some notes, though I haven't looked at all this in a long time:
|
|
Thanks @tpwrules! Those are helpful insights. I have updated the cherry-pick list based on your input. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We'll consider this step 2 of 2(ish) in addressing the larger issue (and opportunity) raised by @tpwrules with:
This PR pulls the code from https://github.com/tpwrules/ODM2DataSharingPortal/tree/wip/batch that is successfully running a private instance of Monitor My Watershed for the Univ. of Memphis.
We should probably merge this PR only after merging this smaller PR that contains the first 4 commits:
@ptomasula, @tpwrules, and @SRGDamia1, as a followup to our call today, I decided to create this PR to the new
tpw_batchfeature branch that I just created fromdevelop.This gives @ptomasula the ability to resolve the merge conflicts, complete the merge, then test it using CDK, without interfering with our current branches.
If merging and testing of the
tpw_batchgoes perfectly, we can then merge intodevelop.If there are challenges with merging or testing, then perhaps we cherry-pick some of the easier commits first.