add: get metadata dict from s3 bucket with last modified date and siz… #69

GHCamille · 2023-09-13T19:33:48Z

add: get metadata dict from s3 bucket with last modified date and size in GB

…e in GB

ghost · 2023-09-13T19:33:50Z

Nice of you to open a PR 🙏
When you're ready and want to get it reviewed, post a comment in this Pull Request with this message: /quack review

GHCamille · 2023-09-15T06:46:30Z

/quack review

ghost

Thanks for the PR 🙏

Other sections

[missing-type-annotations] detected at pyro_risks/utils/s3.py:L137

[intermediate-variable] detected at pyro_risks/utils/s3.py:L150

[intermediate-variable] detected at pyro_risks/utils/s3.py:L151

[intermediate-variable] detected at pyro_risks/utils/s3.py:L152

For a better experience, it's recommended to check the review comments in the tab Files Changed.

Feedback is a gift! Quack will adjust your review style when you add reactions to a review comment

ghost · 2023-09-15T06:46:39Z

pyro_risks/utils/s3.py

@@ -133,3 +133,28 @@ def get_file_metadata(self, object_key):
        obj = self.bucket.Object(object_key)
        metadata = obj.metadata
        return metadata
+
+    def get_files_metadata(self, pattern=None):


Would you mind adding type annotations? 🙏

Pattern explanation 👈
In Python, type annotations are not enforced. But that can be valuable for debugging using mypy, for your collaborators' understanding, and the autocomplete of your IDE. For more details, feel free to check this resource.

Code example

Here is an example of how this is typically handled:

# def is_upper(variable_name): def is_upper(variable_name: str) -> bool:

Quack feedback loop 👍👎
This comment is about [missing-type-annotations]. Add the following reactions on this comment to let us know if:
- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear

ghost · 2023-09-15T06:46:39Z

pyro_risks/utils/s3.py

+        files = []
+        for obj in self.bucket.objects.all():
+            if not pattern or pattern in obj.key:
+                file_name = obj.key


It doesn't look like we need this intermediate variable

Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.

Code example

Here is an example of how this is typically handled:

# def make_upper(input_str: str) -> str: # upper_str = input_str.upper() # return upper_str def make_upper(input_str: str) -> str: return input_str.upper()

Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:
- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear

ghost · 2023-09-15T06:46:39Z

pyro_risks/utils/s3.py

+        for obj in self.bucket.objects.all():
+            if not pattern or pattern in obj.key:
+                file_name = obj.key
+                file_size = round(obj.size * 1.0 / (1024*1000000), 2)


It doesn't look like we need this intermediate variable

Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.

Code example

Here is an example of how this is typically handled:

# def make_upper(input_str: str) -> str: # upper_str = input_str.upper() # return upper_str def make_upper(input_str: str) -> str: return input_str.upper()

Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:
- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear

ghost · 2023-09-15T06:46:39Z

pyro_risks/utils/s3.py

+            if not pattern or pattern in obj.key:
+                file_name = obj.key
+                file_size = round(obj.size * 1.0 / (1024*1000000), 2)
+                file_last_modified = obj.last_modified


It doesn't look like we need this intermediate variable

Pattern explanation 👈
You shouldn't concede readability, but when it's possible, avoid unnecessary memory allocation. For more details, feel free to check this resource.

Code example

Here is an example of how this is typically handled:

# def make_upper(input_str: str) -> str: # upper_str = input_str.upper() # return upper_str def make_upper(input_str: str) -> str: return input_str.upper()

Quack feedback loop 👍👎
This comment is about [intermediate-variable]. Add the following reactions on this comment to let us know if:
- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear

GHCamille · 2023-09-15T06:50:14Z

/quack review

ghost

Thanks for the PR 🙏

Other sections

[missing-type-annotations] detected at pyro_risks/utils/s3.py:L137

For a better experience, it's recommended to check the review comments in the tab Files Changed.

Feedback is a gift! Quack will adjust your review style when you add reactions to a review comment

ghost · 2023-09-15T06:50:21Z

pyro_risks/utils/s3.py

@@ -133,3 +133,25 @@ def get_file_metadata(self, object_key):
        obj = self.bucket.Object(object_key)
        metadata = obj.metadata
        return metadata
+
+    def get_files_metadata(self, pattern=None):


Would you mind adding type annotations? 🙏

Pattern explanation 👈
In Python, type annotations are not enforced. But that can be valuable for debugging using mypy, for your collaborators' understanding, and the autocomplete of your IDE. For more details, feel free to check this resource.

Code example

Here is an example of how this is typically handled:

# def is_upper(variable_name): def is_upper(variable_name: str) -> bool:

Quack feedback loop 👍👎
This comment is about [missing-type-annotations]. Add the following reactions on this comment to let us know if:
- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear

GHCamille · 2023-09-15T06:52:59Z

/quack review

ghost

Thanks for the PR 🙏

Nothing to report from Quack 🥳

GHCamille · 2023-09-15T06:57:00Z

/quack review

ghost

Thanks for the PR 🙏

Other sections

[missing-type-annotations] detected at pyro_risks/utils/s3.py:L78

For a better experience, it's recommended to check the review comments in the tab Files Changed.

Feedback is a gift! Quack will adjust your review style when you add reactions to a review comment

ghost · 2023-09-15T06:57:08Z

pyro_risks/utils/s3.py

@@ -75,7 +75,7 @@ def __init__(
        self.s3 = self.session.resource("s3", endpoint_url=endpoint_url)
        self.bucket = self.s3.Bucket(bucket_name)

-    def upload_file(self, file_path, object_key):
+    def upload_file(self, file_path, object_key: str) -> None:


Would you mind adding type annotations? 🙏

Pattern explanation 👈
In Python, type annotations are not enforced. But that can be valuable for debugging using mypy, for your collaborators' understanding, and the autocomplete of your IDE. For more details, feel free to check this resource.

Code example

Here is an example of how this is typically handled:

# def is_upper(variable_name): def is_upper(variable_name: str) -> bool:

Quack feedback loop 👍👎
This comment is about [missing-type-annotations]. Add the following reactions on this comment to let us know if:
- 👍 that comment was on point
- 👀 that doesn't seem right
- 👎 this isn't important for you right now
- 😕 that explanation wasn't clear

GHCamille · 2023-09-15T06:58:26Z

/quack review

ghost

Thanks for the PR 🙏

Nothing to report from Quack 🥳

juldpnt

Thanks for the additions !

add: get metadata dict from s3 bucket with last modified date and siz…

00ceeeb

…e in GB