Skip to content

Conversation

@simahawk
Copy link
Contributor

@simahawk simahawk commented Mar 11, 2022

Use case:

  • you have a root channel per scope/app (eg: root.edi)
  • you have several sub channels (eg: root.edi.ubl.sales,
    root.edi.gs1.delivery)
  • you want to configure capacity only for the main channel "root.edi"

Before this change, the channel manager falls back on root channel,
and you get flooded w/ warning log entries like "unknown channel....".

However, if you have a specific parent channel configured
it sounds a good idea to use it.

NOTE: using parent_fallback flag is just an attempt to not break places where this feat is not desiderable.
Also, I might fail to see why this behavior was not supported before: any remark is welcomed :)

@guewen can I have your insights please? 🙏

Copy link
Member

@guewen guewen left a comment

Choose a reason for hiding this comment

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

This behavior is exactly the one I would expect in all situations.
Thanks!

Copy link
Member

@flotho flotho left a comment

Choose a reason for hiding this comment

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

LGTM

Use case:

* you have a root channel per scope/app (eg: root.edi)
* you have several sub channels (eg: root.edi.ubl.sales,
root.edi.gs1.delivery)
* you want to configure capacity only for the main channel "root.edi"

Before this change, the channel manager falls back on root channel.
However, if you have a specific parent channel configured
it sounds a good idea to use it.
@simahawk simahawk force-pushed the 14-channel-parent-fallback branch from 6739c3d to 96c9d9b Compare March 11, 2022 16:35
@simahawk
Copy link
Contributor Author

This behavior is exactly the one I would expect in all situations. Thanks!

Cool. I've just dropped the try/except as it's useless now.

Contributing since a while... :)
@simahawk
Copy link
Contributor Author

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-412-by-simahawk-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 5169ac5 into OCA:14.0 Mar 14, 2022
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 43cdb78. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants