Skip to content

Conversation

@rfrederick
Copy link
Contributor

No description provided.

@rfrederick rfrederick marked this pull request as draft December 15, 2025 21:20
@rfrederick rfrederick marked this pull request as ready for review December 15, 2025 21:21
Copy link
Member

@Monviech Monviech left a comment

Choose a reason for hiding this comment

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

Thank you, I can see the issue with this. I think we can simplify it. Nice effort :)

{% if 'asoverride' in neighbor and neighbor.asoverride == '1' %}
neighbor {{ neighbor.address }} as-override
{% endif %}
{% if 'removeprivateas' in neighbor and neighbor.removeprivateas == '1' %}
Copy link
Member

Choose a reason for hiding this comment

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

{%     if 'removeprivateas' in neighbor and neighbor.removeprivateas %}
  neighbor {{ neighbor.address }} {{ neighbor.removeprivateas }}
{%     endif %}

We could replace all of the conditions with these three lines and construct all of neighbor.removeprivateas in the model since there arent too many combinations possible here. This could get rid off the checkbox in favor of only an option field.

</connecttimer>
<defaultoriginate type="BooleanField"/>
<asoverride type="BooleanField"/>
<removeprivateas type="BooleanField"/>
Copy link
Member

@Monviech Monviech Dec 16, 2025

Choose a reason for hiding this comment

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

Remove the booleanfield and create just the option field with that name.

<bestpath type="OptionField">
<OptionValues>
<as_path_confed value="as-path confed">as-path confed</as_path_confed>
<as_path_multipath_relax value="as-path multipath-relax">as-path multipath-relax</as_path_multipath_relax>
<compare_routerid value="compare-routerid">compare-routerid</compare_routerid>
<peer_type_multipath_relax value="peer-type multipath-relax">peer-type multipath-relax</peer_type_multipath_relax>
<aigp>aigp</aigp>
<med_missing_as_worst value="med missing-as-worst">med missing-as-worst</med_missing_as_worst>
</OptionValues>
<Multiple>Y</Multiple>
</bestpath>

If you put all combinations into value its really easy. Without multiple, users can only select one of these combinations.

The value will then be inside {{ neighbor.removeprivateas }}.

</grid_view>
</field>
<field>
<id>neighbor.removeprivateas</id>
Copy link
Member

Choose a reason for hiding this comment

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

Remove one of these, we can do this with only one new field.

@rfrederick
Copy link
Contributor Author

Thank you, I can see the issue with this. I think we can simplify it. Nice effort :)

Thanks for the feedback and the bit of direction on how best to represent the option combinations. I'll modify as needed to present the combinations in a single option field.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants