-
Notifications
You must be signed in to change notification settings - Fork 7
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
[Fix] Overlapping stored flows #44
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is very interesting and the solution is pretty robust and straightforward. I just made two comments concerning the strict/non-strict match trying to collaborate with the proposal.
return True | ||
|
||
|
||
def _match_keys(flow_to_install, stored_flow_dict, flow_to_install_keys): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was extracted to be re-used, but to add support for masked values we'll cover on this issue #24 in the future.
thanks for reviewing, and the second pass Italo and Antonio. |
Fixes #23 that @italovalcy has reported
fix/overlapping_flows
branch directly on the upstream repo to facilitate visualizing diffs since there are upcoming PRs that will depend on this branch here, let's try this out to see if it facilitates for us)Description of the change
_add_flow_store
to overwrite overlapping flows