Introduce filestorage app, support for multiple file backends and store file metadata in the database
#1104
+4,371
−333
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
|
Task linked: QF-2760 Keep the file metadata in the database |
suricactus
force-pushed
the
QF-2760-files
branch
from
895f958 to
59dba4d
Compare
gounux
reviewed
Jan 16, 2025
gounux
reviewed
Jan 17, 2025
suricactus
force-pushed
the
QF-2760-files
branch
from
59dba4d to
a31eaa0
Compare
|
@gounux apologies for the force push, I first rebased and then realized I had to force push. |
gounux
reviewed
Jan 21, 2025
docker-app/qfieldcloud/core/migrations/0080_file_storage_project_and_more.py
Show resolved
Hide resolved
gounux
reviewed
Jan 22, 2025
gounux
reviewed
Jan 27, 2025
There was a problem hiding this comment.
3rd review round.
I faced an error when creating a project with the basic OSM .qgs file :
- project exists in the database
- in the database, the project has
file_storagevalue todefault, but the files are stored in the legacybucket - running the
migrateprojectstoragecommand throw this error :Exception: Cannot migrate to storage "default", project is already stored there!
After looking at the Jobs in QFC Admin, the Process QGIS Project File have Failed status with UNKNOWN Error type :
The Output pre display the error Local files list for project "UUID": empty! :
Feedback pre shows a ProjectFileNotFoundException thrown :
| def next_version(self) -> "FileVersion | None": | ||
| file_version_qs = FileVersion.objects.filter( | ||
| file=self.file, | ||
| uploaded_at__lt=self.uploaded_at, |
There was a problem hiding this comment.
Suggested change
| uploaded_at__lt=self.uploaded_at, | |
| uploaded_at__gt=self.uploaded_at, |
Comment on lines
+20
to
+21
| md5sum = serializers.SerializerMethodField() | ||
| sha256 = serializers.SerializerMethodField() |
| name="is_locked", | ||
| field=models.BooleanField( | ||
| default=False, | ||
| help_text="If set to true, the project is temporarily locked. Locking is internal QFieldCloud mechansism related to file storage migration or other file operations.", |
There was a problem hiding this comment.
Suggested change
| help_text="If set to true, the project is temporarily locked. Locking is internal QFieldCloud mechansism related to file storage migration or other file operations.", | |
| help_text="If set to true, the project is temporarily locked. Locking is internal QFieldCloud mechanism related to file storage migration or other file operations.", |
| is_locked = models.BooleanField( | ||
| _("Is locked"), | ||
| help_text=_( | ||
| "If set to true, the project is temporarily locked. Locking is internal QFieldCloud mechansism related to file storage migration or other file operations." |
There was a problem hiding this comment.
Suggested change
| "If set to true, the project is temporarily locked. Locking is internal QFieldCloud mechansism related to file storage migration or other file operations." | |
| "If set to true, the project is temporarily locked. Locking is internal QFieldCloud mechanism related to file storage migration or other file operations." |
Comment on lines
+26
to
+35
| def whatever(self): | ||
| qs = self.annotate( | ||
| last_version_pk=( | ||
| FileVersion.objects.filter(file=OuterRef("pk")) | ||
| .order_by("-uploaded_at") | ||
| .values("pk")[:1] | ||
| ) | ||
| ) | ||
| qs = qs.prefetch_related("versions") | ||
| return qs |
There was a problem hiding this comment.
Is this strictly mandatory or did you use this for testing ?
Comment on lines
+1151
to
+1152
| # the s3 storage has 1024 bytes (not chars!) limit: https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html | ||
| max_length=1024, |
There was a problem hiding this comment.
Then why use 1024 characters and not the STORAGE_FILENAME_MAX_CHAR_LENGTH settings variable ?
Comment on lines
+284
to
+293
| last_job = project.last_package_job | ||
| if last_job.feedback.get("feedback_version") == "2.0": | ||
| layers = last_job.feedback["outputs"]["qgis_layers_data"]["layers_by_id"] | ||
| else: | ||
| steps = last_job.feedback.get("steps", []) | ||
| layers = ( | ||
| steps[1]["outputs"]["layer_checks"] | ||
| if len(steps) > 2 and steps[1].get("stage", 1) == 2 | ||
| else None | ||
| ) |
There was a problem hiding this comment.
Find it hard to understand what this is supposed to be doing.
Would a comment be welcome ?
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.
This PR introduces a new enhanced way to store filedata. Before just dumped everything to the Object Storage. This made everything clumsy, slow and not flexible.
The plan is to quickly migrate all projects to the new storage and delete the legacy approach.
I have closed #1065, which was the same PR.