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

Device Role and Peer Endpoint Role filters for Peering models #114

Merged
merged 2 commits into from
Aug 11, 2023

Conversation

mzbroch
Copy link
Contributor

@mzbroch mzbroch commented Jul 24, 2023

This PR introduces new filters for Peering model:

  • Device Role
  • Peer Endpoint Role

This will match Peerings by any endpoint side.

TODO:

Copy link
Contributor

@glennmatthews glennmatthews left a comment

Choose a reason for hiding this comment

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

Looks like a good approach to me.

@mzbroch mzbroch marked this pull request as ready for review August 11, 2023 14:57
@mzbroch mzbroch requested a review from glennmatthews August 11, 2023 14:57
@mzbroch mzbroch changed the title [WIP] Device Role and Peer Endpoint Role filters for Peering models Device Role and Peer Endpoint Role filters for Peering models Aug 11, 2023
@@ -224,6 +224,20 @@ class PeeringFilterSet(BaseFilterSet, CreatedUpdatedFilterSet, CustomFieldModelF
label="Device (name)",
)

device_role = django_filters.ModelMultipleChoiceFilter(
field_name="endpoints__routing_instance__device__device_role__name",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to add any prefetch_related/select_related on the Peering list view queryset so as to make this nested lookup not prohibitively expensive at scale?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need to add any prefetch_related/select_related on the Peering list view queryset so as to make this nested lookup not prohibitively expensive at scale?

Yes, this is a good point. I created new issue to revisit peering list view and modify displayed fields! Thanks!

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.

Peering Filter Does Not Work
2 participants