[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r194893128 ## File path: traffic_ops/traffic_ops_golang/crconfig/edgelocations.go ## @@ -49,14 +49,49 @@ and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN') for rows.Next() { cachegroup := "" + primaryCacheID := 0 ttype := "" + var fallbackToClosest *bool latlon := tc.CRConfigLatitudeLongitude{} - if err := rows.Scan(&cachegroup, &ttype, &latlon.Lat, &latlon.Lon); err != nil { + if err := rows.Scan(&cachegroup, &primaryCacheID, &ttype, &latlon.Lat, &latlon.Lon, &fallbackToClosest); err != nil { return nil, nil, errors.New("Error scanning cachegroup: " + err.Error()) } if ttype == RouterTypeName { routerLocs[cachegroup] = latlon } else { + q := `select cachegroup.name from cachegroup_fallbacks +join cachegroup on cachegroup_fallbacks.backup_cg = cachegroup.id +and cachegroup_fallbacks.primary_cg = $1 Review comment: I missed this in my previous review, but this query is missing `order by cachegroup_fallbacks.set_order`. Without this the `set_order` ordering is not enforced. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r193497898 ## File path: traffic_ops/traffic_ops_golang/crconfig/edgelocations.go ## @@ -32,7 +32,7 @@ func makeLocations(cdn string, db *sql.DB) (map[string]tc.CRConfigLatitudeLongit // TODO test whether it's faster to do a single query, joining lat/lon into servers q := ` -select cg.name, t.name as type, cg.latitude, cg.longitude from cachegroup as cg +select cg.name, cg.id, t.name as type, cg.latitude, cg.longitude from cachegroup as cg Review comment: You need to also select cg.fallback_to_closest here and scan the value into a `var fallbackToClosest *bool`. If it's null, set `BackupLocations.FallbackToClosest` below to true. Otherwise, set it to the dereferenced value. Also, I noticed the CRConfig generated sets the value of `fallbackToClosest` to a string rather than a boolean. Since booleans are an actual type in Go, I'm thinking we should set the values to booleans rather than strings and make sure Traffic Router handles that properly. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r193758109 ## File path: traffic_ops/traffic_ops_golang/crconfig/edgelocations.go ## @@ -32,7 +32,7 @@ func makeLocations(cdn string, db *sql.DB) (map[string]tc.CRConfigLatitudeLongit // TODO test whether it's faster to do a single query, joining lat/lon into servers q := ` -select cg.name, t.name as type, cg.latitude, cg.longitude from cachegroup as cg +select cg.name, cg.id, t.name as type, cg.latitude, cg.longitude from cachegroup as cg Review comment: I think the string boolean thing might have stemmed from Perl not having real booleans. Now that it's been migrated to Go, we _could_ use real booleans in the JSON. But since there are a bunch of string bools in the CRConfig already, I guess it's better to be consistent and keep it as a string bool. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r193755892 ## File path: traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouter.java ## @@ -716,17 +722,44 @@ public CacheLocation getCoverageZoneCacheLocation(final String ip, final String return cacheLocation; } + if (cacheLocation != null && cacheLocation.getBackupCacheGroups() != null) { + for (final String cacheGroup : cacheLocation.getBackupCacheGroups()) { + final CacheLocation bkCacheLocation = getCacheRegister().getCacheLocationById(cacheGroup); + if (bkCacheLocation != null && !getSupportingCaches(bkCacheLocation.getCaches(), deliveryService).isEmpty()) { + LOGGER.debug("Got backup CZ cache group " + bkCacheLocation.getId() + " for " + ip + ", ds " + deliveryServiceId); + if (track != null) { + track.setFromBackupCzGroup(true); + } + return bkCacheLocation; + } + } + // track.continueGeo + // will become to false only when backups are configured and (primary group's) fallbackToClosedGeo is configured (non-empty list) to false + // False signals subsequent cacheSelection routine to stop geo based selection. + if (!cacheLocation.isUseClosestGeoLoc()) { + track.continueGeo = false; + return null; + } + } + // We had a hit in the CZF but the name does not match a known cache location. // Check whether the CZF entry has a geolocation and use it if so. - return getClosestCacheLocation(cacheRegister.filterAvailableLocations(deliveryServiceId), networkNode.getGeolocation(), cacheRegister.getDeliveryService(deliveryServiceId)); + final CacheLocation closestCacheLocation = getClosestCacheLocation(cacheRegister.filterAvailableLocations(deliveryServiceId), networkNode.getGeolocation(), cacheRegister.getDeliveryService(deliveryServiceId)); + if (closestCacheLocation != null) { + LOGGER.debug("Got closest CZ cache group " + closestCacheLocation.getId() + " for " + ip + ", ds " + deliveryServiceId); + if (track != null) { + track.setFromBackupCzGroup(true); Review comment: Ok, I can agree with that. It would be useful to track those occurrences as "backup hits". This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r193548798 ## File path: traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/router/TrafficRouter.java ## @@ -716,17 +722,44 @@ public CacheLocation getCoverageZoneCacheLocation(final String ip, final String return cacheLocation; } + if (cacheLocation != null && cacheLocation.getBackupCacheGroups() != null) { + for (final String cacheGroup : cacheLocation.getBackupCacheGroups()) { + final CacheLocation bkCacheLocation = getCacheRegister().getCacheLocationById(cacheGroup); + if (bkCacheLocation != null && !getSupportingCaches(bkCacheLocation.getCaches(), deliveryService).isEmpty()) { + LOGGER.debug("Got backup CZ cache group " + bkCacheLocation.getId() + " for " + ip + ", ds " + deliveryServiceId); + if (track != null) { + track.setFromBackupCzGroup(true); + } + return bkCacheLocation; + } + } + // track.continueGeo + // will become to false only when backups are configured and (primary group's) fallbackToClosedGeo is configured (non-empty list) to false + // False signals subsequent cacheSelection routine to stop geo based selection. + if (!cacheLocation.isUseClosestGeoLoc()) { + track.continueGeo = false; + return null; + } + } + // We had a hit in the CZF but the name does not match a known cache location. // Check whether the CZF entry has a geolocation and use it if so. - return getClosestCacheLocation(cacheRegister.filterAvailableLocations(deliveryServiceId), networkNode.getGeolocation(), cacheRegister.getDeliveryService(deliveryServiceId)); + final CacheLocation closestCacheLocation = getClosestCacheLocation(cacheRegister.filterAvailableLocations(deliveryServiceId), networkNode.getGeolocation(), cacheRegister.getDeliveryService(deliveryServiceId)); + if (closestCacheLocation != null) { + LOGGER.debug("Got closest CZ cache group " + closestCacheLocation.getId() + " for " + ip + ", ds " + deliveryServiceId); + if (track != null) { + track.setFromBackupCzGroup(true); Review comment: Why do we set this to true here? If we reach this point, there were no available backup cachegroups (or none configured), so we shouldn't track it as from a backup cachegroup, right? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r193497898 ## File path: traffic_ops/traffic_ops_golang/crconfig/edgelocations.go ## @@ -32,7 +32,7 @@ func makeLocations(cdn string, db *sql.DB) (map[string]tc.CRConfigLatitudeLongit // TODO test whether it's faster to do a single query, joining lat/lon into servers q := ` -select cg.name, t.name as type, cg.latitude, cg.longitude from cachegroup as cg +select cg.name, cg.id, t.name as type, cg.latitude, cg.longitude from cachegroup as cg Review comment: You need to also select cg.fallback_to_closest here and scan the value into a `var fallbackToClosest *bool`. If it's null, set `BackupLocations.FallbackToClosest` below to false. Otherwise, set it to the dereferenced value. Also, I noticed the CRConfig generated sets the value of `fallbackToClosest` to a string rather than a boolean. Since booleans are an actual type in Go, I'm thinking we should set the values to booleans rather than strings and make sure Traffic Router handles that properly. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r193497898 ## File path: traffic_ops/traffic_ops_golang/crconfig/edgelocations.go ## @@ -32,7 +32,7 @@ func makeLocations(cdn string, db *sql.DB) (map[string]tc.CRConfigLatitudeLongit // TODO test whether it's faster to do a single query, joining lat/lon into servers q := ` -select cg.name, t.name as type, cg.latitude, cg.longitude from cachegroup as cg +select cg.name, cg.id, t.name as type, cg.latitude, cg.longitude from cachegroup as cg Review comment: You need to select cg.fallback_to_closest here and scan the value into a `var fallbackToClosest *bool`. If it's null, set `BackupLocations.FallbackToClosest` below to false. Otherwise, set it to the dereferenced value. Also, I noticed the CRConfig generated sets the value of `fallbackToClosest` to a string rather than a boolean. Since booleans are an actual type in Go, I'm thinking we should set the values to booleans rather than strings and make sure Traffic Router handles that properly. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r191430713 ## File path: lib/go-tc/cachegroupfallback.go ## @@ -20,33 +20,33 @@ package tc * under the License. */ -// A List of CACHEGROUPFALLBACKs Response -// swagger:response CACHEGROUPFALLBACKsResponse +// A List of cachegroupFallbacks Response Review comment: The `c` and `g` should be capitalized to match the rest of the `CacheGroup` structs (capitalizing the first letter is most important in Go because it makes the struct exportable). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r191438939 ## File path: traffic_ops/traffic_ops_golang/crconfig/edgelocations.go ## @@ -49,20 +49,20 @@ and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN') for rows.Next() { cachegroup := "" + primaryCacheID := "" Review comment: kind of a nitpick, but I think this should be `primaryCacheID := 0` to match the int DB type This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r191428268 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); Review comment: For 2, I'm ok with leaving as-is in the Perl, but in the Golang API it will be wrapped in a transaction so that all or none of the requested backups are created/updated. People should not have to double-check our API when getting a 200 success code. For 3 let's just keep it nullable for now. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r191427214 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); Review comment: For 1, yeah I think adding logic so that we get an audit record of what was actually deleted is necessary. Otherwise this log provides no value. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r191425235 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); + return $self->success([]); + } + + if ( ($cachegroup->type->name ne "EDGE_LOC") ) { + &log( $self, "cachegroup should be type EDGE_LOC."); + return $self->success([]); + } + + my $rs_backups = undef; + + if ( defined ($cache_id) && defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id, backup_cg => $fallback_id}, {order_by => 'set_order'}); + } elsif ( defined ($cache_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id}, {order_by => 'set_order'}); + } elsif ( defined ($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ backup_cg => $fallback_id}, {order_by => 'set_order'}); + } + + if ( defined ($rs_backups) && ($rs_backups->count > 0) ) { + my $response; + my $backup_cnt = 0; + while ( my $row = $rs_backups->next ) { + $response->[$backup_cnt]{"cacheGroupId"} = $row->primary_cg->id; + $response->[$backup_cnt]{"cacheGroupName"} = $row->primary_cg->name; + $response->[$backup_cnt]{"fallbackName"} = $row->backup_cg->name; + $response->[$backup_cnt]{"fallbackId"} = $row->backup_cg->id; + $response->[$backup_cnt]{"fallbackOrder"} = $row->set_order; + $backup_cnt++; + } + return
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r191002483 ## File path: traffic_ops/traffic_ops_golang/crconfig/edgelocations.go ## @@ -57,6 +57,36 @@ and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN') if ttype == RouterTypeName { routerLocs[cachegroup] = latlon } else { + primaryCacheId := "" + if err := db.QueryRow(`select id from cachegroup where name = $1`, cachegroup).Scan(&primaryCacheId); err != nil { + return nil, nil, errors.New("Failed while retrieving from cachegroup: " + err.Error()) + } + + dbRows, err := db.Query(`select backup_cg from cachegroup_fallbacks where primary_cg = $1 order by set_order`, primaryCacheId) + + if err != nil { + return nil, nil, errors.New("Error retrieving from cachegroup_fallbacks: " + err.Error()) + } + defer dbRows.Close() + + index := 0 + for dbRows.Next() { + backup_id := "" Review comment: in golang the typical naming style is to use camelcase, i.e. `backupID` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190971957 ## File path: traffic_ops/traffic_ops_golang/crconfig/edgelocations.go ## @@ -57,6 +57,36 @@ and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN') if ttype == RouterTypeName { routerLocs[cachegroup] = latlon } else { + primaryCacheId := "" Review comment: rather than re-querying the cachegroup's ID here, you should just update the query on L35 to also select `cg.id` and scan that into a variable. In fact, you might even be able to eliminate the query on L65 altogether by modifying the query on L35 to include a subquery. Something like: ``` select cg.name, ..., ARRAY(select c.name from cachegroup_fallbacks cgf join cachegroup c on c.id = cgf.backup_cg where cgf.primary_cg = cg.id) ... from ... ``` I haven't tried that query out, but I think it's worth a shot rather than running an extra query per cachegroup. If it's possible, you should be able to scan that result into a slice of strings and just iterate over it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190980948 ## File path: traffic_ops/app/db/migrations/2018052100_cache_group_fallback.sql ## @@ -0,0 +1,38 @@ +/* Review comment: this file will need renamed again because another migration was recently merged This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190961962 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); Review comment: generally the `&log` function should only be used for create/update/delete operations because it inserts a log entry into the database for auditing purposes. This is more of a `$self->app->log->error(...)` message This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190958981 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); Review comment: This should probably say "failed to delete backup configuration" This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190952902 ## File path: traffic_ops/app/db/migrations/2018052100_cache_group_fallback.sql ## @@ -0,0 +1,38 @@ +/* + +Licensed 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. +*/ + +-- +goose Up +-- SQL in section 'Up' is executed when this migration is applied + + +-- cachegroup_fallbacks +CREATE TABLE cachegroup_fallbacks ( +primary_cg bigint, +backup_cg bigint CHECK (primary_cg != backup_cg), +set_order bigint NOT NULL, +CONSTRAINT fk_primary_cg FOREIGN KEY (primary_cg) REFERENCES cachegroup(id) ON DELETE CASCADE, +CONSTRAINT fk_backup_cg FOREIGN KEY (backup_cg) REFERENCES cachegroup(id) ON DELETE CASCADE, +UNIQUE (primary_cg, backup_cg), +UNIQUE (primary_cg, set_order) +); + +ALTER TABLE cachegroup ADD COLUMN fallback_to_closest BOOLEAN DEFAULT TRUE; Review comment: Currently it looks like your perl API implementation protects against clients not supplying `fallbackToClosest` by just inserting the value `true` in that case. However, since the cachegroup API is now written in and handled by the golang API, it will never insert a value there and will always default to `true`. Eventually we'll need to implement it there too so that we can actually set it to `false` when needed. I think columns that have a default could always be made to be `NOT NULL` by having the API insert the default when null, but generally I don't feel too strongly about it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190962982 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); + return $self->success([]); + } + + if ( ($cachegroup->type->name ne "EDGE_LOC") ) { + &log( $self, "cachegroup should be type EDGE_LOC."); + return $self->success([]); + } + + my $rs_backups = undef; + + if ( defined ($cache_id) && defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id, backup_cg => $fallback_id}, {order_by => 'set_order'}); + } elsif ( defined ($cache_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id}, {order_by => 'set_order'}); + } elsif ( defined ($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ backup_cg => $fallback_id}, {order_by => 'set_order'}); + } + + if ( defined ($rs_backups) && ($rs_backups->count > 0) ) { + my $response; + my $backup_cnt = 0; + while ( my $row = $rs_backups->next ) { + $response->[$backup_cnt]{"cacheGroupId"} = $row->primary_cg->id; + $response->[$backup_cnt]{"cacheGroupName"} = $row->primary_cg->name; + $response->[$backup_cnt]{"fallbackName"} = $row->backup_cg->name; + $response->[$backup_cnt]{"fallbackId"} = $row->backup_cg->id; + $response->[$backup_cnt]{"fallbackOrder"} = $row->set_order; + $backup_cnt++; + } + return
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190962922 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); + return $self->success([]); + } + + if ( ($cachegroup->type->name ne "EDGE_LOC") ) { + &log( $self, "cachegroup should be type EDGE_LOC."); + return $self->success([]); + } + + my $rs_backups = undef; + + if ( defined ($cache_id) && defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id, backup_cg => $fallback_id}, {order_by => 'set_order'}); + } elsif ( defined ($cache_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id}, {order_by => 'set_order'}); + } elsif ( defined ($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ backup_cg => $fallback_id}, {order_by => 'set_order'}); + } + + if ( defined ($rs_backups) && ($rs_backups->count > 0) ) { + my $response; + my $backup_cnt = 0; + while ( my $row = $rs_backups->next ) { + $response->[$backup_cnt]{"cacheGroupId"} = $row->primary_cg->id; + $response->[$backup_cnt]{"cacheGroupName"} = $row->primary_cg->name; + $response->[$backup_cnt]{"fallbackName"} = $row->backup_cg->name; + $response->[$backup_cnt]{"fallbackId"} = $row->backup_cg->id; + $response->[$backup_cnt]{"fallbackOrder"} = $row->set_order; + $backup_cnt++; + } + return
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190962185 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); Review comment: see comment on L58 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190962521 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); + return $self->success([]); + } + + if ( ($cachegroup->type->name ne "EDGE_LOC") ) { + &log( $self, "cachegroup should be type EDGE_LOC."); Review comment: see comment on L58 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190959281 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); Review comment: this log line should contain the primary_cg ID from which the backup config was deleted This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190955160 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); + return $self->success([]); Review comment: 404 not found? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190964049 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); + return $self->success([]); + } + + if ( ($cachegroup->type->name ne "EDGE_LOC") ) { + &log( $self, "cachegroup should be type EDGE_LOC."); + return $self->success([]); + } + + my $rs_backups = undef; + + if ( defined ($cache_id) && defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id, backup_cg => $fallback_id}, {order_by => 'set_order'}); + } elsif ( defined ($cache_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id}, {order_by => 'set_order'}); + } elsif ( defined ($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ backup_cg => $fallback_id}, {order_by => 'set_order'}); + } + + if ( defined ($rs_backups) && ($rs_backups->count > 0) ) { + my $response; + my $backup_cnt = 0; + while ( my $row = $rs_backups->next ) { + $response->[$backup_cnt]{"cacheGroupId"} = $row->primary_cg->id; + $response->[$backup_cnt]{"cacheGroupName"} = $row->primary_cg->name; + $response->[$backup_cnt]{"fallbackName"} = $row->backup_cg->name; + $response->[$backup_cnt]{"fallbackId"} = $row->backup_cg->id; + $response->[$backup_cnt]{"fallbackOrder"} = $row->set_order; + $backup_cnt++; + } + return
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190963988 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); + return $self->success([]); + } + + if ( ($cachegroup->type->name ne "EDGE_LOC") ) { + &log( $self, "cachegroup should be type EDGE_LOC."); + return $self->success([]); + } + + my $rs_backups = undef; + + if ( defined ($cache_id) && defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id, backup_cg => $fallback_id}, {order_by => 'set_order'}); + } elsif ( defined ($cache_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id}, {order_by => 'set_order'}); + } elsif ( defined ($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ backup_cg => $fallback_id}, {order_by => 'set_order'}); + } + + if ( defined ($rs_backups) && ($rs_backups->count > 0) ) { + my $response; + my $backup_cnt = 0; + while ( my $row = $rs_backups->next ) { + $response->[$backup_cnt]{"cacheGroupId"} = $row->primary_cg->id; + $response->[$backup_cnt]{"cacheGroupName"} = $row->primary_cg->name; + $response->[$backup_cnt]{"fallbackName"} = $row->backup_cg->name; + $response->[$backup_cnt]{"fallbackId"} = $row->backup_cg->id; + $response->[$backup_cnt]{"fallbackOrder"} = $row->set_order; + $backup_cnt++; + } + return
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190972947 ## File path: traffic_ops/traffic_ops_golang/crconfig/edgelocations.go ## @@ -57,6 +57,36 @@ and (st.name = 'REPORTED' or st.name = 'ONLINE' or st.name = 'ADMIN_DOWN') if ttype == RouterTypeName { routerLocs[cachegroup] = latlon } else { + primaryCacheId := "" + if err := db.QueryRow(`select id from cachegroup where name = $1`, cachegroup).Scan(&primaryCacheId); err != nil { + return nil, nil, errors.New("Failed while retrieving from cachegroup: " + err.Error()) + } + + dbRows, err := db.Query(`select backup_cg from cachegroup_fallbacks where primary_cg = $1 order by set_order`, primaryCacheId) Review comment: you can use a `join` here and remove the extra query on L79: ``` select cgf.backup_cg, c.name from cachegroup_fallbacks cgf join cachegroup c on c.id = cgf.backup_cg ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190950254 ## File path: traffic_ops/app/db/migrations/2018052100_cache_group_fallback.sql ## @@ -0,0 +1,38 @@ +/* + +Licensed 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. +*/ + +-- +goose Up +-- SQL in section 'Up' is executed when this migration is applied + + +-- cachegroup_fallbacks +CREATE TABLE cachegroup_fallbacks ( +primary_cg bigint, Review comment: If there's no DB op with these values as null, then that's all the more reason to do it. Generally we should always be using `NOT NULL` wherever it makes sense. That way if somebody changes the API implementation, they can't accidentally make it so that it accepts null values. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190955096 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); Review comment: shouldn't this return a 500 alert not a success? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190962154 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); Review comment: see comment on L58 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190958237 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); + return $self->success([]); + } + + if ( ($cachegroup->type->name ne "EDGE_LOC") ) { + &log( $self, "cachegroup should be type EDGE_LOC."); + return $self->success([]); + } + + my $rs_backups = undef; + + if ( defined ($cache_id) && defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id, backup_cg => $fallback_id}, {order_by => 'set_order'}); + } elsif ( defined ($cache_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id}, {order_by => 'set_order'}); + } elsif ( defined ($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ backup_cg => $fallback_id}, {order_by => 'set_order'}); + } + + if ( defined ($rs_backups) && ($rs_backups->count > 0) ) { + my $response; + my $backup_cnt = 0; + while ( my $row = $rs_backups->next ) { + $response->[$backup_cnt]{"cacheGroupId"} = $row->primary_cg->id; + $response->[$backup_cnt]{"cacheGroupName"} = $row->primary_cg->name; + $response->[$backup_cnt]{"fallbackName"} = $row->backup_cg->name; + $response->[$backup_cnt]{"fallbackId"} = $row->backup_cg->id; + $response->[$backup_cnt]{"fallbackOrder"} = $row->set_order; + $backup_cnt++; + } + return
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190948333 ## File path: lib/go-tc/cachegroupfallback.go ## @@ -0,0 +1,55 @@ +package tc + + +/* + * 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. + */ + +// A List of CACHEGROUPFALLBACKs Response +// swagger:response CACHEGROUPFALLBACKsResponse +// in: body +type CACHEGROUPFALLBACKsResponse struct { Review comment: camelcase should be used to keep in style with the rest of the go code: `CachegroupFallbacksResponse` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190963209 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); + return $self->success([]); + } + + if ( ($cachegroup->type->name ne "EDGE_LOC") ) { + &log( $self, "cachegroup should be type EDGE_LOC."); + return $self->success([]); + } + + my $rs_backups = undef; + + if ( defined ($cache_id) && defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id, backup_cg => $fallback_id}, {order_by => 'set_order'}); + } elsif ( defined ($cache_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id}, {order_by => 'set_order'}); + } elsif ( defined ($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ backup_cg => $fallback_id}, {order_by => 'set_order'}); + } + + if ( defined ($rs_backups) && ($rs_backups->count > 0) ) { + my $response; + my $backup_cnt = 0; + while ( my $row = $rs_backups->next ) { + $response->[$backup_cnt]{"cacheGroupId"} = $row->primary_cg->id; + $response->[$backup_cnt]{"cacheGroupName"} = $row->primary_cg->name; + $response->[$backup_cnt]{"fallbackName"} = $row->backup_cg->name; + $response->[$backup_cnt]{"fallbackId"} = $row->backup_cg->id; + $response->[$backup_cnt]{"fallbackOrder"} = $row->set_order; + $backup_cnt++; + } + return
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r190962381 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,287 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); + return $self->success([]); + } + + if ( ($cachegroup->type->name ne "EDGE_LOC") ) { + &log( $self, "cachegroup should be type EDGE_LOC."); + return $self->success([]); + } + + my $rs_backups = undef; + + if ( defined ($cache_id) && defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id, backup_cg => $fallback_id}, {order_by => 'set_order'}); + } elsif ( defined ($cache_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id}, {order_by => 'set_order'}); + } elsif ( defined ($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ backup_cg => $fallback_id}, {order_by => 'set_order'}); + } + + if ( defined ($rs_backups) && ($rs_backups->count > 0) ) { + my $response; + my $backup_cnt = 0; + while ( my $row = $rs_backups->next ) { + $response->[$backup_cnt]{"cacheGroupId"} = $row->primary_cg->id; + $response->[$backup_cnt]{"cacheGroupName"} = $row->primary_cg->name; + $response->[$backup_cnt]{"fallbackName"} = $row->backup_cg->name; + $response->[$backup_cnt]{"fallbackId"} = $row->backup_cg->id; + $response->[$backup_cnt]{"fallbackOrder"} = $row->set_order; + $backup_cnt++; + } + return
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r189971490 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,278 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); + return $self->success([]); + } + + if ( ($cachegroup->type->name ne "EDGE_LOC") ) { + &log( $self, "cachegroup should be type EDGE_LOC."); + return $self->success([]); + } + + my $rs_backups = undef; + + if ( defined ($cache_id) && defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id, backup_cg => $fallback_id}, {order_by => 'set_order'}); + } elsif ( defined ($cache_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id}, {order_by => 'set_order'}); + } elsif ( defined ($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ backup_cg => $fallback_id}, {order_by => 'set_order'}); + } + + if ( defined ($rs_backups) && ($rs_backups->count > 0) ) { + my $response; + my $backup_cnt = 0; + while ( my $row = $rs_backups->next ) { + $response->[$backup_cnt]{"cacheGroupId"} = $row->primary_cg->id; + $response->[$backup_cnt]{"cacheGroupName"} = $row->primary_cg->name; + $response->[$backup_cnt]{"fallbackName"} = $row->backup_cg->name; + $response->[$backup_cnt]{"fallbackId"} = $row->backup_cg->id; + $response->[$backup_cnt]{"fallbackOrder"} = $row->set_order; + $backup_cnt++; + } + return
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r189935669 ## File path: docs/source/development/traffic_ops.rst ## @@ -367,264 +367,3 @@ Traffic Ops accesses each extension through the addition of a URL route as a cus Development Configuration -- To incorporate any custom Extensions during development set your PERL5LIB with any number of directories with the understanding that the PERL5LIB search order will come into play, so keep in mind that top-down is how your code will be located. Once Perl locates your custom route or Perl package/class it 'pins' on that class or Mojo Route and doesn't look any further, which allows for the developer to *override* Traffic Ops functionality. - -API Review comment: Did you mean to remove this section? Seems totally unrelated to this change and maybe pulled in accidentally when rebasing This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r189955184 ## File path: traffic_ops/app/db/migrations/2018052100_cache_group_fallback.sql ## @@ -0,0 +1,38 @@ +/* + +Licensed 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. +*/ + +-- +goose Up +-- SQL in section 'Up' is executed when this migration is applied + + +-- cachegroup_fallbacks +CREATE TABLE cachegroup_fallbacks ( +primary_cg bigint, Review comment: Should `primary_cg` and `backup_cg` both be `NOT NULL` as well? I'm not sure null values would make sense there. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r189963481 ## File path: traffic_ops/app/lib/API/CachegroupFallback.pm ## @@ -0,0 +1,278 @@ +package API::CachegroupFallback; +# +# +# Licensed 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. +# +# +# +# a note about locations and cachegroups. This used to be "Location", before we had physical locations in 12M. Very confusing. +# What used to be called a location is now called a "cache group" and location is now a physical address, not a group of caches working together. +# + +# JvD Note: you always want to put Utils as the first use. Sh*t don't work if it's after the Mojo lines. +use UI::Utils; +use Mojo::Base 'Mojolicious::Controller'; +use Data::Dumper; +use JSON; +use MojoPlugins::Response; +use Validate::Tiny ':all'; + +sub delete { + my $self = shift; + my $cache_id = $self->param('cacheGroupId'); + my $fallback_id = $self->param('fallbackId'); + my $params = $self->req->json; + my $rs_backups = undef; + + if ( !&is_oper($self) ) { + return $self->forbidden(); + } + + if ( defined ($cache_id) && defined($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id , backup_cg => $fallback_id} ); + } elsif (defined ($cache_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { primary_cg => $cache_id} ); + } elsif (defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search( { backup_cg => $fallback_id} ); + } + + if ( ($rs_backups->count > 0) ) { + my $del_records = $rs_backups->delete(); + if ($del_records) { + &log( $self, "Backup configuration DELETED", "APICHANGE"); + return $self->success_message("Backup configuration DELETED"); + } else { + return $self->alert( "Backup configuration DELETED." ); + } + } else { + &log( $self, "No backup Cachegroups found"); + return $self->not_found(); + } +} + +sub show { + my $self = shift; + my $cache_id = $self->param("cacheGroupId"); + my $fallback_id = $self->param("fallbackId"); + my $id = $cache_id ? $cache_id : $fallback_id; + + #only integers + if ( $id !~ /^\d+?$/ ) { + &log( $self, "No such Cachegroup id $id"); + return $self->success([]); + } + + my $cachegroup = $self->db->resultset('Cachegroup')->search( { id => $id } )->single(); + if ( !defined($cachegroup) ) { + &log( $self, "No such Cachegroup $id"); + return $self->success([]); + } + + if ( ($cachegroup->type->name ne "EDGE_LOC") ) { + &log( $self, "cachegroup should be type EDGE_LOC."); + return $self->success([]); + } + + my $rs_backups = undef; + + if ( defined ($cache_id) && defined ($fallback_id)) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id, backup_cg => $fallback_id}, {order_by => 'set_order'}); + } elsif ( defined ($cache_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ primary_cg => $cache_id}, {order_by => 'set_order'}); + } elsif ( defined ($fallback_id) ) { + $rs_backups = $self->db->resultset('CachegroupFallback')->search({ backup_cg => $fallback_id}, {order_by => 'set_order'}); + } + + if ( defined ($rs_backups) && ($rs_backups->count > 0) ) { + my $response; + my $backup_cnt = 0; + while ( my $row = $rs_backups->next ) { + $response->[$backup_cnt]{"cacheGroupId"} = $row->primary_cg->id; + $response->[$backup_cnt]{"cacheGroupName"} = $row->primary_cg->name; + $response->[$backup_cnt]{"fallbackName"} = $row->backup_cg->name; + $response->[$backup_cnt]{"fallbackId"} = $row->backup_cg->id; + $response->[$backup_cnt]{"fallbackOrder"} = $row->set_order; + $backup_cnt++; + } + return
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r189958107 ## File path: traffic_ops/app/db/migrations/2018052100_cache_group_fallback.sql ## @@ -0,0 +1,38 @@ +/* + +Licensed 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. +*/ + +-- +goose Up +-- SQL in section 'Up' is executed when this migration is applied + + +-- cachegroup_fallbacks +CREATE TABLE cachegroup_fallbacks ( +primary_cg bigint, +backup_cg bigint CHECK (primary_cg != backup_cg), +set_order bigint NOT NULL, +CONSTRAINT fk_primary_cg FOREIGN KEY (primary_cg) REFERENCES cachegroup(id) ON DELETE CASCADE, +CONSTRAINT fk_backup_cg FOREIGN KEY (backup_cg) REFERENCES cachegroup(id) ON DELETE CASCADE, +UNIQUE (primary_cg, backup_cg), +UNIQUE (primary_cg, set_order) +); + +ALTER TABLE cachegroup ADD COLUMN fallback_to_closest BOOLEAN DEFAULT TRUE; Review comment: This should also probably be NOT NULL? Might save us from a bunch of unnecessary null checks when reading from the DB. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup
rawlinp commented on a change in pull request #2029: [Issue 1907] TO API for backup edge cachegroup URL: https://github.com/apache/incubator-trafficcontrol/pull/2029#discussion_r189953754 ## File path: traffic_ops/app/lib/API/Cachegroup.pm ## @@ -57,6 +57,7 @@ sub index { "lastUpdated" => $row->last_updated, "parentCachegroupId"=> $row->parent_cachegroup_id, "parentCachegroupName" => ( defined $row->parent_cachegroup_id ) ? $idnames{ $row->parent_cachegroup_id } : undef, + "fallbackToClosest" => \$row->fallback_to_closest, Review comment: I saw API reference documentation for the new `cachegroup_fallbacks` endpoint, but the existing cachegroup API reference documentation should also be updated to show this new `fallbackToClosest` field. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services