You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2021/03/25 18:36:00 UTC
[GitHub] [trafficcontrol] pbivrell commented on a change in pull request #5606: Blueprint for phys_location change
pbivrell commented on a change in pull request #5606:
URL: https://github.com/apache/trafficcontrol/pull/5606#discussion_r601748797
##########
File path: blueprints/enhanced-physlocations.md
##########
@@ -0,0 +1,352 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+# Enhanced physlocations
+
+## Problem Description
+
+This blue print proposes two semi major alterations to the physical location model with ATC
+
+* Multiple addresses per physical location
+* Multiple POC per physical location
+
+This improvement allows for TC to remain the source of truth for metadata about server locations. Specifically this enhanced functionality will allow storage/retrieval of information required for CDN hardware mailing, location specific contact management. The below solution is extensible without unnecessary complexity enabling our users to define useful server location metadata to fit their needs.
+
+## Proposed Change
+<!--
+*How* will this be implemented (at a high level)?
+-->
+
+This functionality involves adjusting the representation of physical locations by altering the following components.
+
+* Traffic Ops Database representation of physical locations
+* TO Rest API physical location response value
+* TP physical location data insertion views
+
+### Traffic Portal Impact
+<!--
+*How* will this impact Traffic Portal?
+What new UI changes will be required?
+Will entirely new pages/views be necessary?
+Will a new field be added to an existing form?
+How will the user interact with the new UI changes?
+-->
+
+The change should only expand the number of fields required to input `N` new physical location and points of contact. New visuals could be added for displaying this additional information but I think it would clutter the table view.
+
+### Traffic Ops Impact
+<!--
+*How* will this impact Traffic Ops (at a high level)?
+-->
+
+The primarily changes take place within Traffic ops. There will be an API response change for the physical location endpoints, a database table addition and modification.
+
+#### REST API Impact
+<!--
+*How* will this impact the Traffic Ops REST API?
+
+What new endpoints will be required?
+How will existing endpoints be changed?
+What will the requests and responses look like?
+What fields are required or optional?
+What are the defaults for optional fields?
+What are the validation constraints?
+-->
+
+This modification will update the `GET` response and `POST` request fields for the `/phys_locations` endpoint. It should be a breaking change modifying the names of existing fields and adding new fields.
+
+The `GET` response will be modified as follows.
+
+```
+{ "response": [
+ {
+ "comments": "",
+ "id": 2,
+ "lastUpdated": "2018-12-05 17:50:58+00",
+ "name": "CDN_in_a_Box",
+ "regionId": 1,
+ "region": "Washington, D.C",
+ "shortName": "ciab",
+ "addresses": [ {
+ "address1": "1600 Pennsylvania Avenue NW",
+ "address2": "",
+ "address3": "",
+ "address4": "",
+ "locality": "Washington",
+ "administrativeArea": "DC",
+ "postalCode": "20500",
+ "longitude": -77.03650834403494,
+ "latitude": 38.897846677690815,
+ "type": "primary"
+ }],
+ "contacts": [ {
+ "contact": "Joe Schmo",
+ "method": "phone",
+ "value": "800-555-1234",
+ "comments": "Security Guard",
+ "priority": 1,
+ "purpose": "controls access to physical location"
+ }]
+ }
+]}
+```
+
+The `POST` request will look very similar.
+
+```
+{
+ "comments": "",
+ "name": "CDN_in_a_Box",
+ "regionId": 1,
+ "region": "Washington, D.C",
+ "shortName": "ciab",
+ "addresses": [ {
+ "address1": "1600 Pennsylvania Avenue NW",
+ "address2": "",
+ "address3": "",
+ "address4": "",
+ "locality": "Washington",
+ "administrativeArea": "DC",
+ "postalCode": "20500",
+ "longitude": -77.03650834403494,
+ "latitude": 38.897846677690815,
+ "type": "primary"
+ }],
+ "contacts": [ {
+ "contact": "Joe Schmo",
+ "method": "phone",
+ "value": "800-555-1234",
+ "comments": "Security Guard",
+ "priority": 1,
+ "purpose": "controls access to physical location"
+ }]
+}
+
+```
+
+For the POST request body it will be important to validate the longitude and latitude as sensible values.
+
+Some of the address lines can be optional.
+
+The type and method fields could probably be enumeration types. But I am not sure we understand/want to enforce all of the possible use cases so it may be important to limit the input text length.
+
+It may be desirable to update the address and contacts without re-POSTing all of the above data. I am not sure there is enough necessity to create this endpoint.
+
+#### Client Impact
+<!--
+*How* will this impact Traffic Ops REST API clients (Go, Python, Java)?
+
+If new endpoints are required, will corresponding client methods be added?
+-->
+
+I am unaware of the client use of phys_location but they will most likely need to be updated to support the new formats.
+
+#### Data Model / Database Impact
+<!--
+*How* will this impact the Traffic Ops data model?
+*How* will this impact the Traffic Ops database schema?
+
+What changes to the lib/go-tc structs will be required?
+What new tables and columns will be required?
+How will existing tables and columns be changed?
+What are the column data types and modifiers?
+What are the FK references and constraints?
+-->
+
+Firstly the phys_location table will be altered. Simply dropping many of to be duplicated columns.
+
+
+```
+ Table "traffic_ops.phys_location"
+ Column | Type | Collation | Nullable | Default
+------------------+--------------------------+-----------+----------+---------
+id | bigint | | NOT NULL |
+name | text | | NOT NULL |
+short_name | text | | NOT NULL |
+region | bigint | | NOT NULL |
+last_updated | timestamp with time zone | | NOT NULL | now()
+
+Indexes:
+ idx_89655_primary PRIMARY KEY (id)
+ idx_89655_fk_phys_location_region_idx ON phys_location USING btree (region)
+ idx_89655_name_unique ON phys_location USING btree (name);
+ idx_89655_short_name_unique ON phys_location USING btree (short_name);
+Foreign-key constraints:
+ fk_phys_location_region FOREIGN KEY (region) REFERENCES region(id);
+
+```
+
+Specifically the following columns have been dropped.
+
+* address
+* city
+* state
+* zip
+* poc
+* phone
+* email
+* comments
+
+The first new table stores sets of addresses. This adds 3 extra address lines and changes the terminology for (state -> adminstrativeArea, city -> locality, zip -> postalCode), this is more friendly to international addresses. The other notable change is the addition of the type column. This is to be used for differentiating address. All locations in the phy_location table should have at least one full address of type `primary`, others could include a hardware mailing address or a contact address
+
+```
+ Table "traffic_ops.location_addresses"
+ Column | Type | Collation | Nullable | Default
+------------------+--------------------------+-----------+----------+---------
+id | bigint | | NOT NULL |
+addressLine1 | text | | NOT NULL |
+addressLine2 | text | | NOT NULL |
+addressLine3 | text | | NOT NULL |
+addressLine4 | text | | NOT NULL |
+country | text | | NOT NULL |
+administrativeArea| text | | NOT NULL |
+longitude | bigfloat | | NOT NULL |
+latitude | bigfloat | | NOT NULL |
Review comment:
I am on the fence about this. It is pretty easy to find latitude and longitude information for a location. And it minimizes ambiguity potential in addresses, as well as could be useful for visualizing locations. Those are all "nice to have" but maybe not something that should be enforced.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org