Skip to content

fix: Adds missing reference to the endpoint_url for the S3 proxy#294

Merged
frgfm merged 2 commits intomainfrom
fix-endpoint-url
Oct 18, 2023
Merged

fix: Adds missing reference to the endpoint_url for the S3 proxy#294
frgfm merged 2 commits intomainfrom
fix-endpoint-url

Conversation

@Nastaliss
Copy link
Copy Markdown
Collaborator

@Nastaliss Nastaliss commented Oct 17, 2023

The endpoint_url reference in the method to generate file download URL is missing. This PR fixes that

@codecov
Copy link
Copy Markdown

codecov bot commented Oct 17, 2023

Codecov Report

Merging #294 (f7e16d1) into main (4c04a9d) will not change coverage.
Report is 2 commits behind head on main.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##             main     #294   +/-   ##
=======================================
  Coverage   95.17%   95.17%           
=======================================
  Files          66       66           
  Lines        1576     1576           
=======================================
  Hits         1500     1500           
  Misses         76       76           
Flag Coverage Δ
client 100.00% <ø> (ø)
unittests 94.94% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/app/services/bucket/s3.py 44.73% <0.00%> (ø)

@frgfm
Copy link
Copy Markdown
Member

frgfm commented Oct 18, 2023

@Nastaliss Would you mind adding a PR description and creating an issue referencing all the PRs on that topic please? 🙏

@frgfm frgfm self-requested a review October 18, 2023 09:44
@frgfm frgfm added type: fix Something isn't working module: services labels Oct 18, 2023
@frgfm frgfm added this to the 0.2.0 milestone Oct 18, 2023
Copy link
Copy Markdown
Member

@frgfm frgfm left a comment

Choose a reason for hiding this comment

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

No need for an extra instance attribute, I'll edit the PR 👍

Comment thread src/app/services/bucket/s3.py Outdated
self, region: str, endpoint_url: str, access_key: str, secret_key: str, bucket_name: str, proxy_url: str
) -> None:
_session = boto3.Session(access_key, secret_key, region_name=region)
self.endpoint_url = endpoint_url
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need we have

self._s3.meta.endpoint_url

@frgfm frgfm changed the title fix: add class attribute fix: Adds missing reference to the endpoint_url for the S3 proxy Oct 18, 2023
@frgfm frgfm merged commit d3b244d into main Oct 18, 2023
@frgfm frgfm deleted the fix-endpoint-url branch October 18, 2023 10:06
@Nastaliss
Copy link
Copy Markdown
Collaborator Author

issue

@frgfm frgfm mentioned this pull request Oct 19, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: fix Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants