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

CommunitySetMatchExprVarCollector: do not crash on collecting unsupported structures #6750

Merged
merged 1 commit into from
Mar 18, 2021

Conversation

dhalperi
Copy link
Member

Instead, defer that crashing to validation time. This enables SearchRoutePolicies to
run on devices where only some of the route policies can be successfully analyzed.

…rted structures

Instead, defer that crashing to validation time. This enables SearchRoutePolicies to
run on devices where only some of the route policies can be successfully analyzed.
@dhalperi dhalperi requested a review from millstein March 18, 2021 22:06
@batfish-bot
Copy link

This change is Reviewable

Copy link
Contributor

@millstein millstein left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dhalperi)

a discussion (no related file):
This looks great, thanks. Note that the same issue arises with UnsupportedOperationExceptions in CommunityMatchExprVarCollector.


@codecov
Copy link

codecov bot commented Mar 18, 2021

Codecov Report

Merging #6750 (62fbd45) into master (9142036) will increase coverage by 0.00%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             master    #6750    +/-   ##
==========================================
  Coverage     69.51%   69.52%            
- Complexity    35913    35945    +32     
==========================================
  Files          3011     3013     +2     
  Lines        153087   153192   +105     
  Branches      18404    18405     +1     
==========================================
+ Hits         106424   106511    +87     
- Misses        38064    38073     +9     
- Partials       8599     8608     +9     
Impacted Files Coverage Δ Complexity Δ
...sh/minesweeper/bdd/CommunitySetMatchExprToBDD.java 93.61% <ø> (ø) 17.00 <0.00> (ø)
...communities/CommunitySetMatchExprVarCollector.java 94.11% <50.00%> (+10.78%) 10.00 <1.00> (+1.00)
...g/batfish/datamodel/flow/IncomingSessionScope.java 84.61% <0.00%> (-15.39%) 7.00% <0.00%> (-1.00%)
...src/main/java/org/batfish/coordinator/PoolMgr.java 55.95% <0.00%> (-3.58%) 14.00% <0.00%> (-1.00%)
...a/org/batfish/datamodel/acl/AclLineMatchExprs.java 82.30% <0.00%> (-0.89%) 49.00% <0.00%> (-1.00%)
...ain/java/org/batfish/coordinator/WorkQueueMgr.java 70.93% <0.00%> (-0.59%) 90.00% <0.00%> (-1.00%)
...src/main/java/org/batfish/coordinator/WorkMgr.java 75.65% <0.00%> (-0.16%) 247.00% <0.00%> (ø%)
...tfish/representation/cisco/CiscoConfiguration.java 84.87% <0.00%> (-0.15%) 426.00% <0.00%> (-1.00%)
.../org/batfish/dataplane/ibdp/BgpRoutingProcess.java 84.79% <0.00%> (-0.12%) 150.00% <0.00%> (-1.00%)
.../java/org/batfish/representation/fortios/Zone.java 100.00% <0.00%> (ø) 12.00% <0.00%> (+1.00%)
... and 7 more

Copy link
Member Author

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

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

Dismissed @millstein from a discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

a discussion (no related file):

Previously, millstein (Todd Millstein) wrote…

This looks great, thanks. Note that the same issue arises with UnsupportedOperationExceptions in CommunityMatchExprVarCollector.

Done.


Copy link
Member Author

@dhalperi dhalperi left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

a discussion (no related file):

Previously, dhalperi (Dan Halperin) wrote…

Done.

Done was a weird auto-add. Ack, next work.


@dhalperi dhalperi merged commit a4a197e into batfish:master Mar 18, 2021
@dhalperi dhalperi deleted the regex-stuff branch March 18, 2021 22:24
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.

None yet

3 participants