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

Consolidate the storage implementations + Remove libcloud dependency #640

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

rzvoncek
Copy link
Contributor

@rzvoncek rzvoncek commented Sep 15, 2023

Fixes #618 .

@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #640 (4ca32ba) into master (6f9452e) will decrease coverage by 0.17%.
The diff coverage is 93.46%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #640      +/-   ##
==========================================
- Coverage   80.97%   80.80%   -0.17%     
==========================================
  Files          53       52       -1     
  Lines        4652     4398     -254     
  Branches      661      628      -33     
==========================================
- Hits         3767     3554     -213     
+ Misses        857      816      -41     
  Partials       28       28              
Files Changed Coverage Δ
medusa/storage/__init__.py 94.04% <85.71%> (+0.46%) ⬆️
medusa/storage/abstract_storage.py 91.85% <89.65%> (+2.48%) ⬆️
medusa/storage/google_storage.py 93.63% <90.90%> (-1.25%) ⬇️
medusa/storage/s3_base_storage.py 93.95% <92.30%> (+3.14%) ⬆️
medusa/backup_node.py 87.37% <100.00%> (-0.07%) ⬇️
medusa/restore_node.py 79.18% <100.00%> (-0.10%) ⬇️
medusa/storage/azure_storage.py 99.01% <100.00%> (+5.72%) ⬆️
medusa/storage/local_storage.py 98.66% <100.00%> (+1.79%) ⬆️
medusa/storage/s3_rgw.py 71.42% <100.00%> (+7.14%) ⬆️

@rzvoncek rzvoncek force-pushed the radovan/remove-libcloud branch 2 times, most recently from 162b9e9 to 4ca32ba Compare September 15, 2023 12:25
@rzvoncek rzvoncek changed the title Radovan/remove libcloud Consolidate the storage implementations + Remove libcloud dependency Sep 15, 2023
@rzvoncek rzvoncek marked this pull request as ready for review September 15, 2023 14:48
@rzvoncek rzvoncek force-pushed the radovan/remove-libcloud branch 4 times, most recently from eef8bbf to 4c052a9 Compare September 18, 2023 14:58
@rzvoncek
Copy link
Contributor Author

Also pushed a change to fix the bug reported by SonarCloud.

@@ -1,4 +1,4 @@
FROM ubuntu:18.04 as base
k8s/Dockerfile-azure FROM ubuntu:18.04 as base
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm actally not sure how this sneakedi n.

Copy link
Contributor

Choose a reason for hiding this comment

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

lol, we're removing this file anyway.

Copy link
Contributor

@adejanovski adejanovski left a comment

Choose a reason for hiding this comment

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

Fantastic job!
We're good for a merge (with two refactoring tickets that need to be created)

@@ -1,4 +1,4 @@
FROM ubuntu:18.04 as base
k8s/Dockerfile-azure FROM ubuntu:18.04 as base
Copy link
Contributor

Choose a reason for hiding this comment

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

lol, we're removing this file anyway.

logging.debug('Loading storage_provider: {}'.format(self._config.storage_provider))
if self._config.storage_provider == Provider.GOOGLE_STORAGE:
if self._config.storage_provider.lower() == 'google_storage':
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: we should create a refactoring ticket to create new constants for the storage providers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

name=object_key,
data=data,
overwrite=True,
max_concurrency=4,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: we had settled on 16 ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@sonarcloud
Copy link

sonarcloud bot commented Sep 19, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 3 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
0.1% 0.1% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@rzvoncek rzvoncek merged commit e653bc6 into master Sep 19, 2023
7 of 8 checks passed
@rzvoncek rzvoncek deleted the radovan/remove-libcloud branch September 22, 2023 11:42
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.

Storage: Prevent leaking storage backend connections
2 participants