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

Wip check fors3 #269

Closed
wants to merge 15 commits into from
Closed

Wip check fors3 #269

wants to merge 15 commits into from

Conversation

mr-shekhar
Copy link
Collaborator

No description provided.

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Sep 8, 2022

Sourcery Code Quality Report

❌  Merging this PR will decrease code quality in the affected files by 1.19%.

Quality metrics Before After Change
Complexity 2.29 ⭐ 2.67 ⭐ 0.38 👎
Method Length 81.63 🙂 87.81 🙂 6.18 👎
Working memory 4.04 ⭐ 4.39 ⭐ 0.35 👎
Quality 72.06% 🙂 70.87% 🙂 -1.19% 👎
Other metrics Before After Change
Lines 536 576 40
Changed files Quality Before Quality After Quality Change
nuxeo/handlers/s3.py 71.91% 🙂 69.12% 🙂 -2.79% 👎
tests/test_upload_s3.py 72.19% 🙂 72.39% 🙂 0.20% 👍

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation
nuxeo/handlers/s3.py ChunkUploaderS3._state 10 🙂 177 😞 9 🙂 52.64% 🙂 Try splitting into smaller methods
tests/test_upload_s3.py test_upload_chunked_resume 3 ⭐ 227 ⛔ 55.76% 🙂 Try splitting into smaller methods
nuxeo/handlers/s3.py ChunkUploaderS3.iter_upload 7 ⭐ 223 ⛔ 7 🙂 56.53% 🙂 Try splitting into smaller methods
tests/test_upload_s3.py test_upload_chunked_error 0 ⭐ 155 😞 69.00% 🙂 Try splitting into smaller methods
tests/test_upload_s3.py test_upload_chunked 0 ⭐ 135 😞 72.27% 🙂 Try splitting into smaller methods

Legend and Explanation

The emojis denote the absolute quality of the code:

  • ⭐ excellent
  • 🙂 good
  • 😞 poor
  • ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.


Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Help us improve this quality report!

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 3 committers have signed the CLA.

✅ mr-shekhar
❌ Sweta Yadav
❌ swetayadav1


Sweta Yadav seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codecov
Copy link

codecov bot commented Sep 8, 2022

Codecov Report

Merging #269 (cf7948e) into master (0c9e86d) will decrease coverage by 0.29%.
The diff coverage is 69.56%.

@@            Coverage Diff             @@
##           master     #269      +/-   ##
==========================================
- Coverage   96.58%   96.29%   -0.30%     
==========================================
  Files          29       29              
  Lines        1787     1808      +21     
==========================================
+ Hits         1726     1741      +15     
- Misses         61       67       +6     
Flag Coverage Δ
unit 96.29% <69.56%> (-0.30%) ⬇️

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

Impacted Files Coverage Δ
nuxeo/handlers/s3.py 89.58% <69.56%> (-3.10%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@@ -50,14 +50,20 @@ def __init__(self, *args, **kwargs):
def _create_s3_client(self, s3_info):
# type: (Dict[str, Any]) -> BaseClient
"""Create the S3 client."""
return boto3.Session().client(
print("checking for _create_s3_client")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove print statements

UP_AMAZON_S3,
aws_access_key_id=s3_info["awsSecretKeyId"],
aws_secret_access_key=s3_info["awsSecretAccessKey"],
aws_session_token=s3_info["awsSessionToken"],
endpoint_url=s3_info.get("endpoint") or None,
config=self._s3_config,
)
print(s3_instance)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove print statements

@@ -182,13 +188,24 @@ def _state(self):
part_number_marker = 0

while "there are parts":
print(">>>there are parts")
print(self.s3_client)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove print statements

info = self.s3_client.list_parts(
Bucket=self.bucket,
Key=self.key,
UploadId=self.batch.multiPartUploadId,
PartNumberMarker=part_number_marker,
MaxParts=self._max_parts,
)

print("Parts are >>>>>>>>>>>>>>")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove print statements

@@ -204,6 +221,8 @@ def _state(self):
data_packs.append({"ETag": part["ETag"], "PartNumber": index})
uploaded_chunks.append(index)

print("uploaded_chunks ")
print(uploaded_chunks)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove print statements

with mock_s3():
print(">>>mock_s3>>>>")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove print statement

client = boto3.client(UP_AMAZON_S3, region_name="eu-west-1")
print(client)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove print statement

CreateBucketConfiguration={"LocationConstraint": "eu-west-1"},
)
except Exception as ex:
print(ex)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove print statements, if req, put proper logging


# Output the bucket names
print('Existing buckets:')
for bucket in response['Buckets']:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

why is this loop required?

try:
ChunkUploaderS3(server.uploads, batch, blob, 1024 * 1024 * 5, s3_client=s3)
except Exception as ex:
print("Exception in ChunkUploaderS3")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove print statements. If req. put proper logging

@mr-shekhar mr-shekhar closed this Oct 11, 2022
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.

None yet

3 participants