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

filemanager: attributes route and misc CORS changes #515

Merged
merged 5 commits into from
Aug 27, 2024

Conversation

mmalenic
Copy link
Member

@mmalenic mmalenic commented Aug 23, 2024

Closes #495
Depends on #460

Changes

  • Add a new route under /api/v1/s3/attributes which accepts arbitrary parameters that can be a shortcut to querying by top-level attribute properties.

Misc

  • Make the CORS allow methods and allow headers configurable.
  • Mirror the content-type and content-encoding from the client when creating presigned URLs.

@mmalenic mmalenic self-assigned this Aug 23, 2024
@mmalenic mmalenic added filemanager an issue relating to the filemanager feature New feature labels Aug 23, 2024
Base automatically changed from fix/filemanager-perf-queries to main August 25, 2024 23:47
@mmalenic mmalenic linked an issue Aug 25, 2024 that may be closed by this pull request
@victorskl
Copy link
Member

Reviewing...

Copy link
Member

@williamputraintan williamputraintan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the fix!

compose.yml Outdated
@@ -71,6 +71,7 @@ services:
- DATABASE_URL=postgresql://orcabus:orcabus@db:5432/filemanager
- RUST_LOG=debug
- FILEMANAGER_API_CORS_ALLOW_ORIGINS=${FILEMANAGER_API_CORS_ALLOW_ORIGINS:-http://localhost:8400}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this could be http://localhost:3000, as we set the local port for UI to 3000.

@williamputraintan
Copy link
Member

As a side note when the UI is deployed, we still need to adjust the cors from the s3 bucket side 😅

@mmalenic
Copy link
Member Author

As a side note when the UI is deployed, we still need to adjust the cors from the s3 bucket side 😅

This would be outside of the OrcaBus stack right, as the bucket configuration is not controlled in this repo?

@victorskl
Copy link
Member

Yup.

Will; please feel free to PR for required/desired CORS setting at bucket TF.
You can take ILMN CORS setting as starter - around these lines.

@victorskl
Copy link
Member

Good to merge?

…ager-misc

# Conflicts:
#	lib/workload/stateless/stacks/filemanager/filemanager-migrate-lambda/src/main.rs
@mmalenic mmalenic merged commit cff2b40 into main Aug 27, 2024
5 checks passed
@mmalenic mmalenic deleted the refactor/filemanager-misc branch August 27, 2024 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature filemanager an issue relating to the filemanager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

filemanager: add way to shorten attribute API querying
4 participants