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

feat: created functions to get multiple signed URLs. #105

Merged
merged 4 commits into from
Jun 10, 2023

Conversation

dishwad
Copy link
Contributor

@dishwad dishwad commented Jun 8, 2023

Addresses issues:
#104

@dishwad dishwad marked this pull request as draft June 8, 2023 02:18
@dishwad dishwad marked this pull request as ready for review June 8, 2023 02:20
storage3/_async/file_api.py Outdated Show resolved Hide resolved
storage3/_async/file_api.py Outdated Show resolved Hide resolved
storage3/types.py Outdated Show resolved Hide resolved
storage3/types.py Outdated Show resolved Hide resolved
@dishwad dishwad requested a review from anand2312 June 8, 2023 14:50
@anand2312 anand2312 linked an issue Jun 9, 2023 that may be closed by this pull request
Copy link
Contributor

@anand2312 anand2312 left a comment

Choose a reason for hiding this comment

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

LGTM

@anand2312
Copy link
Contributor

@dishwad run poetry run pre-commit install locally to install pre-commit hooks so that black is automatically run on every commit - that's what seems to be failing in CI right now 😄

Also you don't have to manually write the code for the sync client, we generate that using unasync - poetry run unasync storage3 and poetry run unasync tests generates sync code for us 😅 I should probably document that somewhere

@anand2312 anand2312 requested a review from J0 June 9, 2023 04:16
@dishwad
Copy link
Contributor Author

dishwad commented Jun 9, 2023

Ok I removed the code for the sync client and the sync test. I ran poetry run pre-commit install locally and black is passing for me locally.

@J0
Copy link
Collaborator

J0 commented Jun 10, 2023

Hm might be the case that poetry version of black doesn't tally with the pre commit hook config. Former is 23.3.0 and latter is 23.1.0. Let me bump - might need to trouble you to rerun

@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2023

Codecov Report

Patch coverage: 84.21% and project coverage change: -1.48 ⚠️

Comparison is base (03c5b03) 87.18% compared to head (2a3d129) 85.71%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #105      +/-   ##
==========================================
- Coverage   87.18%   85.71%   -1.48%     
==========================================
  Files          12       12              
  Lines         320      371      +51     
==========================================
+ Hits          279      318      +39     
- Misses         41       53      +12     
Impacted Files Coverage Δ
storage3/_async/file_api.py 85.22% <78.57%> (-1.26%) ⬇️
storage3/_sync/file_api.py 85.22% <78.57%> (-1.26%) ⬇️
storage3/types.py 95.34% <100.00%> (+1.23%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@anand2312
Copy link
Contributor

anand2312 commented Jun 10, 2023

Odd - the pre-commit hook for black was reformatting tests/_async/test_client.py on CI but ignoring it locally. Running black manually on that file seems to have fixed it.
Thanks for the PR @dishwad!

@anand2312 anand2312 merged commit 2c5e2fc into supabase:main Jun 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add create_signed_urls function to return multiple signed URLs
4 participants