juniper parse and partially convert condition #6658
Conversation
arifogel
commented
Feb 25, 2021
•
arifogel
commented
- parse, convert policy-statement from condition
- parse, partially convert policy-options condition if-route-exists
- TODO: VI design and implementation for checking route existence in RP
- just return false for if-route-exists for now
- TODO: VI design and implementation for checking route existence in RP
- parse, convert policy-statement from condition
- parse, partially convert policy-options condition if-route-exists
- TODO: VI design and implementation for HasRoute
- HasRoute just returns false for now
|
This change is |
There was a problem hiding this comment.
Reviewed 15 of 15 files at r1.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @arifogel)
projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_policy_options.g4, line 41 at r1 (raw file):
(
please put paren on next line. Better to conform with existing standards. If you want to refactor entire grammar, that can be done, but later.
projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java, line 6168 at r1 (raw file):
tIfRouteExists().setPrefix(toPrefix(ctx.prefix));
can't check two prefixes?
projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java, line 6173 at r1 (raw file):
xists().setTable(unquote(ctx.name.getText()));
can't check two tables?
projects/batfish/src/main/java/org/batfish/representation/juniper/Condition.java, line 30 at r1 (raw file):
} private final String _name;
projects/batfish/src/main/java/org/batfish/representation/juniper/PsFromCondition.java, line 5 at r1 (raw file):
n.base.Objects;
use java objects, not guava objects. (MoreObjects if guava).
projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/routing_policy/expr/HasRoute.java, line 15 at r1 (raw file):
@ParametersAreNonnullByDefault public final class HasRoute extends BooleanExpr {
This looks like a never-implemented IOS-XR construct. Let's not add a new dependency on it, but rather just convert as False for now?
There was a problem hiding this comment.
Reviewable status: 10 of 14 files reviewed, 3 unresolved discussions (waiting on @dhalperi)
projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_policy_options.g4, line 41 at r1 (raw file):
Previously, dhalperi (Dan Halperin) wrote…
(please put paren on next line. Better to conform with existing standards. If you want to refactor entire grammar, that can be done, but later.
ok, done
projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java, line 6168 at r1 (raw file):
Previously, dhalperi (Dan Halperin) wrote…
tIfRouteExists().setPrefix(toPrefix(ctx.prefix));can't check two prefixes?
nope
projects/batfish/src/main/java/org/batfish/grammar/flatjuniper/ConfigurationBuilder.java, line 6173 at r1 (raw file):
Previously, dhalperi (Dan Halperin) wrote…
xists().setTable(unquote(ctx.name.getText()));can't check two tables?
nope
projects/batfish/src/main/java/org/batfish/representation/juniper/Condition.java, line 30 at r1 (raw file):
Previously, dhalperi (Dan Halperin) wrote…
done
projects/batfish/src/main/java/org/batfish/representation/juniper/PsFromCondition.java, line 5 at r1 (raw file):
Previously, dhalperi (Dan Halperin) wrote…
n.base.Objects;use java objects, not guava objects. (MoreObjects if guava).
oops! fixed.
projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/routing_policy/expr/HasRoute.java, line 15 at r1 (raw file):
Previously, dhalperi (Dan Halperin) wrote…
This looks like a never-implemented IOS-XR construct. Let's not add a new dependency on it, but rather just convert as False for now?
ok. reverted here, and changed where it was used.
Codecov Report
@@ Coverage Diff @@
## master #6658 +/- ##
============================================
+ Coverage 73.50% 73.51% +0.01%
- Complexity 36592 36628 +36
============================================
Files 2930 2933 +3
Lines 147316 147422 +106
Branches 17765 17771 +6
============================================
+ Hits 108278 108371 +93
- Misses 30511 30517 +6
- Partials 8527 8534 +7 |
There was a problem hiding this comment.
Reviewed 5 of 5 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @arifogel)
projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_policy_options.g4, line 42 at r2 (raw file):
: CONDITION name = junos_name (
Throughout? Or leave it.
There was a problem hiding this comment.
Reviewable status: 13 of 14 files reviewed, 1 unresolved discussion (waiting on @dhalperi)
projects/batfish/src/main/antlr4/org/batfish/grammar/flatjuniper/FlatJuniper_policy_options.g4, line 42 at r2 (raw file):
Previously, dhalperi (Dan Halperin) wrote…
Throughout? Or leave it.
missed that other one.
WRT other style changes, I'd prefer to leave the one-line rules/tokens that are super simple unless you strongly object prior to a full refactor.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status:complete! all files reviewed, all discussions resolved
