[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_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_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 ( !_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) {
+ ( $self, "Backup configuration DELETED",
"APICHANGE");
+ return $self->success_message("Backup configuration
DELETED");
+ } else {
+ return $self->alert( "Backup configuration DELETED." );
+ }
+ } else {
+ ( $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+?$/ ) {
+ ( $self, "No such Cachegroup id $id");
+ return $self->success([]);
+ }
+
+ my $cachegroup = $self->db->resultset('Cachegroup')->search( { id =>
$id } )->single();
+ if ( !defined($cachegroup) ) {
+ ( $self, "No such Cachegroup $id");
+ return $self->success([]);
+ }
+
+ if ( ($cachegroup->type->name ne "EDGE_LOC") ) {
+ ( $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 $self->success(
[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(); 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_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 ( !_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) {
+ ( $self, "Backup configuration DELETED",
"APICHANGE");
+ return $self->success_message("Backup configuration
DELETED");
+ } else {
+ return $self->alert( "Backup configuration DELETED." );
+ }
+ } else {
+ ( $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+?$/ ) {
+ ( $self, "No such Cachegroup id $id");
+ return $self->success([]);
+ }
+
+ my $cachegroup = $self->db->resultset('Cachegroup')->search( { id =>
$id } )->single();
+ if ( !defined($cachegroup) ) {
+ ( $self, "No such Cachegroup $id");
+ return $self->success([]);
+ }
+
+ if ( ($cachegroup->type->name ne "EDGE_LOC") ) {
+ ( $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 $self->success(
[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 ( !_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) {
+ ( $self, "Backup configuration DELETED",
"APICHANGE");
+ return $self->success_message("Backup configuration
DELETED");
+ } else {
+ return $self->alert( "Backup configuration DELETED." );
+ }
+ } else {
+ ( $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+?$/ ) {
+ ( $self, "No such Cachegroup id $id");
+ return $self->success([]);
+ }
+
+ my $cachegroup = $self->db->resultset('Cachegroup')->search( { id =>
$id } )->single();
+ if ( !defined($cachegroup) ) {
+ ( $self, "No such Cachegroup $id");
+ return $self->success([]);
+ }
+
+ if ( ($cachegroup->type->name ne "EDGE_LOC") ) {
+ ( $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 $self->success(
[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 ( !_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) {
+ ( $self, "Backup configuration DELETED",
"APICHANGE");
+ return $self->success_message("Backup configuration
DELETED");
+ } else {
+ return $self->alert( "Backup configuration DELETED." );
+ }
+ } else {
+ ( $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+?$/ ) {
+ ( $self, "No such Cachegroup id $id");
+ return $self->success([]);
+ }
+
+ my $cachegroup = $self->db->resultset('Cachegroup')->search( { id =>
$id } )->single();
+ if ( !defined($cachegroup) ) {
+ ( $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_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 ( !_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) {
+ ( $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_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(); 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_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 ( !_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) {
+ ( $self, "Backup configuration DELETED",
"APICHANGE");
+ return $self->success_message("Backup configuration
DELETED");
+ } else {
+ return $self->alert( "Backup configuration DELETED." );
+ }
+ } else {
+ ( $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+?$/ ) {
+ ( $self, "No such Cachegroup id $id");
+ return $self->success([]);
+ }
+
+ my $cachegroup = $self->db->resultset('Cachegroup')->search( { id =>
$id } )->single();
+ if ( !defined($cachegroup) ) {
+ ( $self, "No such Cachegroup $id");
+ return $self->success([]);
+ }
+
+ if ( ($cachegroup->type->name ne "EDGE_LOC") ) {
+ ( $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 $self->success(
[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 ( !_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) {
+ ( $self, "Backup configuration DELETED",
"APICHANGE");
+ return $self->success_message("Backup configuration
DELETED");
+ } else {
+ return $self->alert( "Backup configuration DELETED." );
+ }
+ } else {
+ ( $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+?$/ ) {
+ ( $self, "No such Cachegroup id $id");
+ return $self->success([]);
+ }
+
+ my $cachegroup = $self->db->resultset('Cachegroup')->search( { id =>
$id } )->single();
+ if ( !defined($cachegroup) ) {
+ ( $self, "No such Cachegroup $id");
+ return $self->success([]);
+ }
+
+ if ( ($cachegroup->type->name ne "EDGE_LOC") ) {
+ ( $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 $self->success(
[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_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_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 ( !_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) {
+ ( $self, "Backup configuration DELETED",
"APICHANGE");
+ return $self->success_message("Backup configuration
DELETED");
+ } else {
+ return $self->alert( "Backup configuration DELETED." );
+ }
+ } else {
+ ( $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+?$/ ) {
+ ( $self, "No such Cachegroup id $id");
+ return $self->success([]);
+ }
+
+ my $cachegroup = $self->db->resultset('Cachegroup')->search( { id =>
$id } )->single();
+ if ( !defined($cachegroup) ) {
+ ( $self, "No such Cachegroup $id");
+ return $self->success([]);
+ }
+
+ if ( ($cachegroup->type->name ne "EDGE_LOC") ) {
+ ( $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 $self->success(
[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
