peeringdb / peeringdb

Server code for https://www.peeringdb.com/
BSD 2-Clause "Simplified" License
340 stars 111 forks source link

Add field "state" to {carrier,ix,net}fac object #1601

Closed arnoldnipper closed 3 weeks ago

arnoldnipper commented 3 weeks ago

Is your feature request related to a problem? Please describe. Currently, only city and country is added as gratuitous information. However, for countries with a strong state structure this isn't sufficient to identify the location or to aggregate.

Who is affected by the problem? All users living in countries with strong state structure

What is the impact? They have to do an extra lookup

Are there security concerns? no

Are there privacy concerns? no

Describe the solution you'd like When looking up city and country also lookup state

Do you think this feature will require a formal design? no, as adding "state" is straightforward

Describe alternatives you've considered There are no easy alternatives

Could this feature request need support from the Admin Committee? no

What is the proposed priority? As implementation is lowest hanging fruit and would comfort many users it should be implemented with the next release

martinhannigan commented 3 weeks ago

On Sat, Apr 20, 2024 at 2:23 PM Arnold Nipper @.***> wrote:

Is your feature request related to a problem? Please describe. Currently, only city and country is added as gratuitous information. However, for countries with a strong state structure this isn't sufficient to identify the location or to aggregate.

Interesting.

Who is affected by the problem? All users living in countries with strong state structure

What is the impact? They have to do an extra lookup

Sounds like the cost impact is negligible - and we've removed enough recently to theoretically make this net/net.

[ clip ]

Describe the solution you'd like

When looking up city and country also lookup state

Do you think this feature will require a formal design? no, as adding "state" is straightforward

Describe alternatives you've considered There are no easy alternatives

Agreed. However, there are more details to be considered. From an impact perspective, does this deepend hiding remote peering? Unless owners/operators can "clean" their fac records for truth, this may compound that obfuscation further. Similar thought for virtual IX.

:watching

Message ID: @.***>

arnoldnipper commented 3 weeks ago

Sounds like the cost impact is negligible

It isn't. If we add the "state" field this needs one extra lookup when copying this gratuitous information from the fac object. When we don't add the "state" field, every user who wants to know information about the state has to do an extra lookup.

And I would argue the other way round. When building the *fac objects we simply forgot to add the "state" field. Hence, it's more a bug fix than a feature request :)

arnoldnipper commented 3 weeks ago

From an impact perspective, does this deepend hiding remote peering?

None of the *fac has to do with peering. Peering relationship is reflected in the netixlan object. Augmenting this object with facility information as proposed in #607 will help discovering remote peering. Give a +1 there!

martinhannigan commented 3 weeks ago

On Sat, Apr 20, 2024 at 3:03 PM Arnold Nipper @.***> wrote:

Sounds like the cost impact is negligible

Let me add the complete statement and explain:

Sounds like the cost impact is negligible - and we've removed enough recently to theoretically make this net/net.

By this I mean we've removed other fields and items that require lookups which should counterbalance an increase in cost for this. However, we should still analyze it to make sure it's effective. I would be surprised if it were cost prohibitive.

It isn't. If we add the "state" field this needs one extra lookup when

copying this gratuitous information from the fac object. When we don't add the "state" field, every user who wants to know information about the state has to do an extra lookup.

Which is an extra step for the user and then [ related cost discussion ].

And I would argue the other way round. When building the *fac objects we simply forgot to add the "state" field. Hence, it's more a bug fix than a feature request :)

What appears to have happened, at least with most United States oriented facility objects, is represented in attached images. As I've said previously, when something is lacking in PeeringDB users find a way and use what makes sense to address a deficiency. We're on the same page for at least the facility object.

[image: Screenshot 2024-04-20 at 3.34.21 PM.png]

Message ID: @.***>

[image: Screenshot 2024-04-20 at 3.34.37 PM.png]

arnoldnipper commented 3 weeks ago

Closing in favour of #1199

job commented 3 weeks ago

@martinhannigan screenshot is missing