8000 Add `azureblockblob_base_path` config by ruaridhw · Pull Request #6669 · celery/celery · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add azureblockblob_base_path config #6669

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Mar 13, 2021
Merged

Add azureblockblob_base_path config #6669

merged 12 commits into from
Mar 13, 2021

Conversation

ruaridhw
Copy link
Contributor
@ruaridhw ruaridhw commented Mar 9, 2021 •
edited
Loading

Description

  • Allow for a basepath such as 'FolderName/' within the Azure container
  • Port across the same pattern from S3Backend

Add azureblockblob_base_path config
6e93325 
- Allow for a basepath such as 'FolderName/' within the Azure container
@codecov
Copy link
codecov bot commented Mar 9, 2021 •
edited
Loading

Codecov Report

Merging #6669 (be22700) into master (1ff7059) will increase coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6669   +/-   ##
=======================================
  Coverage   70.46%   70.47%           
=======================================
  Files         138      138           
  Lines       16495    16494    -1     
  Branches     2073     2073           
=======================================
  Hits        11624    11624           
+ Misses       4669     4668    -1     
  Partials      202      202
Flag Coverage Δ
unittests 70.47% <0.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
celery/app/defaults.py 97.33% <ø> (ø)
celery/backends/azureblockblob.py 36.20% <0.00%> (-0.64%) ⬇️
celery/backends/redis.py 83.23% <0.00%> (+0.23%) ⬆️
celery/exceptions.py 32.60% <0.00%> (+0.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ff7059...be22700. Read the comment docs.

auvipy
auvipy requested changes Mar 9, 2021
View reviewed changes
celery/backends/azureblockblob.py
@@ -43,6 +43,8 @@ def __init__(self,
container_name or
conf["azureblockblob_container_name"])

self.base_path = conf.get('azureblockblob_base_path', '')
Copy link
Member

Choose a reason for hiding this comment

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

changes suggested needs unit tests coverage

Copy link
Contributor Author
@ruaridhw ruaridhw Mar 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Added unit tests however I do this by setting .base_path, not by using conf. Happy to use the latter but I'd just need to redesign the tests in t/unit/backends/test_azureblockblob.py

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've added more tests to cover using conf

@lgtm-com
Copy link
lgtm-com bot commented Mar 9, 2021

This pull request introduces 1 alert when merging 7655909 into 1ff7059 - view on LGTM.com

new alerts:

  • 1 for Non-exception in 'except' clause

@lgtm-com
Copy link
lgtm-com bot commented Mar 9, 2021

This pull request introduces 2 alerts when merging 5b6daff into 1ff7059 - view on LGTM.com

new alerts:

  • 1 for Non-exception in 'except' clause
  • 1 for Module is imported with 'import' and 'import from'

@lgtm-com
Copy link
lgtm-com bot commented Mar 10, 2021

This pull request introduces 1 alert when merging af8d1e6 into 1ff7059 - view on LGTM.com

new alerts:

  • 1 for Non-exception in 'except' clause

@ruaridhw ruaridhw requested a review from auvipy March 10, 2021 09:47
@ruaridhw
Copy link
Contributor Author

@auvipy back to you. I can't see how the codecov job can be fixed to be honest... Clearly L46 is covered by tests now.

auvipy and others added 4 commits March 11, 2021 13:13
@auvipy
Update celery/backends/azureblockblob.py
784871b 
use a fstring

Co-authored-by: Omer Katz <omer.drow@gmail.com>
@auvipy
Update celery/backends/azureblockblob.py
cdab5bb 
use a fstring

Co-authored-by: Omer Katz <omer.drow@gmail.com>
@auvipy
Update celery/backends/azureblockblob.py
272b379 
use a fstring

Co-authored-by: Omer Katz <omer.drow@gmail.com>
@auvipy
Update docs/userguide/configuration.rst
be22700 
Co-authored-by: Omer Katz <omer.drow@gmail.com>
@ruaridhw
Copy link
Contributor Author

@thedrow @auvipy, I believe this is ready for merging or another look. As mentioned above, I have added tests covering the whole class so I can't see what would allow the codecov job to pass.

@ruaridhw ruaridhw requested a review from thedrow March 12, 2021 14:48
@auvipy auvipy merged commit 47af168 8000 into celery:master Mar 13, 2021
@ruaridhw ruaridhw deleted the feat/azure_base_path branch March 14, 2021 17:20
jeyrce pushed a commit to jeyrce/celery that referenced this pull request Aug 25, 2021
@ruaridhw @auvipy @jeyrce
Add azureblockblob_base_path config (celery#6669)
d9ded1a 
* Add `azureblockblob_base_path` config

- Allow for a basepath such as 'FolderName/' within the Azure container

* Docs for `azureblockblob_base_path`

* Add Azure basepath to defaults

* Add unit tests of Azure base path

* Update Contributors

* Add `versionadded` to docs of Azure basepath

* Fix example path

* Add tests for base_path conf

* Update celery/backends/azureblockblob.py

use a fstring

Co-authored-by: Omer Katz <omer.drow@gmail.com>

* Update celery/backends/azureblockblob.py

use a fstring

Co-authored-by: Omer Katz <omer.drow@gmail.com>

* Update celery/backends/azureblockblob.py

use a fstring

Co-authored-by: Omer Katz <omer.drow@gmail.com>

* Update docs/userguide/configuration.rst

Co-authored-by: Omer Katz <omer.drow@gmail.com>

Co-authored-by: Asif Saif Uddin <auvipy@gmail.com>
Co-authored-by: Omer Katz <omer.drow@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@auvipy auvipy auvipy approved these changes

Assignees
No one assigned
Labels
Component: Azure Block Blob Results Backend PR Type: Enhancement Status: Has Test Coverage ✔
Projects
None yet
Milestone
5.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@ruaridhw @auvipy
0