Add fanout to filesystem #1499
Add fanout to filesystem #1499
Conversation
karajan1001
force-pushed
the
add_fanout_to_filesystem
branch
from
c1d251e
to
ae7201c
Compare
|
This pull request introduces 1 alert when merging 06fb0e5 into 22adaaa - view on LGTM.com new alerts:
|
There was a problem hiding this comment.
Thank you for contributing the tests. Please address @matusvalo's code review in #1280 and we can move ahead with merging this PR.
filesystem transport lacks of fanout support. 1. Add fanout support to filesystem transport. 2. Add a unit test for it.
karajan1001
force-pushed
the
add_fanout_to_filesystem
branch
from
06fb0e5
to
4961e26
Compare
for more information, see https://pre-commit.ci
|
BTW, the modifications between this PR and the #1280 are not a small one. So maybe the previous review will play only a limited role? |
|
I have added feedback but:
|
karajan1001
force-pushed
the
add_fanout_to_filesystem
branch
from
cc044b5
to
23fef51
Compare
1. Remove all of refactoring work 2. make the test pass
karajan1001
force-pushed
the
add_fanout_to_filesystem
branch
from
23fef51
to
aab3d6a
Compare
karajan1001
force-pushed
the
add_fanout_to_filesystem
branch
from
8c0881d
to
3640453
Compare
karajan1001
force-pushed
the
add_fanout_to_filesystem
branch
from
3640453
to
a3a4a3d
Compare
|
|
||
| @contextmanager | ||
| def _get_exchange_file_obj(self, exchange, mode="rb"): | ||
| file = self.control_folder / f"{exchange}.exchange" |
There was a problem hiding this comment.
Use pathlib for path management. but didn't expand this to other parts.
|
LGTM for me except two small points:
|
There was a problem hiding this comment.
control_folder is not documented in docstring in the beginning of the file.
I am not 100% sure whether @cachedproperty on top of control_folder is good idea. We had several problems with cached_property. Is there any reason to have it there? Does it make sense to have there only @property instead?
|
Thank you @karajan1001 ! |
|
Excuse me, is there any schedule for the next release? |
|
Not yet but I'm working on it. |
|
@thedrow are there any updates on when the next kombu release will be available? We are getting close to being ready to release our own downstream product that depends on this feature |
wait next week please |
|
@auvipy any updates? |
|
this week or week end for sure |
|
@auvipy I see that the 5.3 milestone for celery+kombu keeps getting pushed back, is there any chance we could just get a limited 5.2.x kombu release with this patch included? |
no, 5.3a1 this week. |
Looks like #1280 is stale?
filesystem transport lacks of fanout support.