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

[Fix] Overlapping stored flows #44

Merged
merged 15 commits into from
Dec 8, 2021
Merged

[Fix] Overlapping stored flows #44

merged 15 commits into from
Dec 8, 2021

Conversation

viniarck
Copy link
Member

Fixes #23 that @italovalcy has reported

  • This PR is same as PR [Fix] Overlapping stored flows #42 (but I'm using the 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

  • Augmented _add_flow_store to overwrite overlapping flows
  • Added unit test to cover it

main.py Outdated Show resolved Hide resolved
@viniarck viniarck requested a review from ajoaoff November 29, 2021 16:59
Copy link

@italovalcy italovalcy left a 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.

main.py Outdated Show resolved Hide resolved
tests/unit/test_main.py Show resolved Hide resolved
@viniarck viniarck requested a review from italovalcy December 2, 2021 15:50
return True


def _match_keys(flow_to_install, stored_flow_dict, flow_to_install_keys):
Copy link
Member Author

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.

@viniarck
Copy link
Member Author

viniarck commented Dec 8, 2021

thanks for reviewing, and the second pass Italo and Antonio.

@viniarck viniarck merged commit d1e10d1 into master Dec 8, 2021
@viniarck viniarck deleted the fix/overlapping_flows branch December 8, 2021 14:39
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.

Duplicated flows are created when using overlapping requests
3 participants