Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upload organisation logo in S3. #1008

Merged
merged 9 commits into from
Nov 26, 2023
Merged

Upload organisation logo in S3. #1008

merged 9 commits into from
Nov 26, 2023

Conversation

nrjadkry
Copy link
Member

@nrjadkry nrjadkry commented Nov 24, 2023

Changes made

  1. Added S3_BUCKET_NAME_BASEMAPS and S3_BUCKET_NAME_OVERLAYS in the config file
  2. Upload organisation logo in the fmtm-data bucket in the s3.

I have used S3_DOWNLOAD_ROOT to get the image path. @spwoodcock is it okay?

Todo

  1. We might need to migrate existing images of organisation logo which are in the container to the s3 and update the url path in db.
  2. Use this url to display the logo in the frontend.

@spwoodcock
Copy link
Member

Looks good, but I intentionally removed those two env vars from the setup. I consolidated into a single bucket defined by S3_BUCKET_NAME.

To have all the data in the same bucket, we need a consistent directory structure.

What do you think about:

ROOT/{ORG_ID}/{PROJECT_ID}?

So logos would go in
ROOT/{ORG_ID}/logo.png

(if we consistently named it logo.png we could even remove the database record for the file)

For the PR I am working on for project export it would be:
ROOT/{ORG_ID}/{PROJECT_ID}/export.zip

Basemaps:

ROOT/{ORG_ID}/{PROJECT_ID}/basemaps/all.pmtiles

ROOT/{ORG_ID}/{PROJECT_ID}/basemaps/{TASK_ID}.mbtiles

@nrjadkry
Copy link
Member Author

Sounds good, we can store data in this format ROOT/{ORG_ID}/{PROJECT_ID} .
But, we should be extra careful in maintaining this consitency.
I will remove those two env var from config file.

@spwoodcock
Copy link
Member

Yes using S3_DOWNLOAD_ROOT for the download url is perfect 👍

I took the opportunity to fix the linting errors on the file for pre-commit.
Also updated the error handling, with organization rollback on exception.

The logo always goes to /{org_id}/logo.png for every organization, so we consistently know where the logo is.

@spwoodcock spwoodcock merged commit 0e7fd36 into development Nov 26, 2023
5 of 6 checks passed
@spwoodcock spwoodcock deleted the upload-to-s3 branch November 26, 2023 15:59
nischalstha9 pushed a commit to naxa-developers/fmtm that referenced this pull request Dec 8, 2023
* added basemaps and overlays buckets in the config file

* added organisation logo in s3

* logo url updated in the create organisation api

* remove: basemap and bucket from config file

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix: add forward slash to s3_path if missing

* refactor: move organization logo upload to func

* fix: org creation exception handling, org id rollback

* refactor: organization_crud linting errors

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: spwoodcock <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Related to backend code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants