rawlinp commented on a change in pull request #5974:
URL: https://github.com/apache/trafficcontrol/pull/5974#discussion_r661823511
##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -207,6 +207,91 @@ func BuildWhereAndOrderByAndPagination(parameters
map[string]string, queryParams
return whereClause, orderBy, paginationClause, queryValues, errs
}
+// CheckIfCurrentUserCanModifyCDN checks if the current user has the lock on
the cdn that the requested operation is to be performed on.
+// This will succeed if the either there is no lock by any user on the CDN, or
if the current user has the lock on the CDN.
+func CheckIfCurrentUserCanModifyCDN(tx *sql.Tx, cdn, user string) (error,
error, int) {
+ query := `SELECT username, soft FROM cdn_lock WHERE cdn=$1`
+ var userName string
+ var soft bool
+ rows, err := tx.Query(query, cdn)
+ if err != nil {
+ if errors.Is(err, sql.ErrNoRows) {
+ return nil, nil, http.StatusOK
+ }
+ return nil, errors.New("querying cdn_lock for user " + user + "
and cdn " + cdn + ": " + err.Error()), http.StatusInternalServerError
+ }
+ defer rows.Close()
+ for rows.Next() {
+ err = rows.Scan(&userName, &soft)
+ if err != nil {
+ return nil, errors.New("scanning cdn_lock for user " +
user + " and cdn " + cdn + ": " + err.Error()), http.StatusInternalServerError
+ }
+ }
+ if userName != "" && user != userName && !soft {
+ return errors.New("user " + userName + " currently has a hard
lock on cdn " + cdn), nil, http.StatusForbidden
+ }
+ return nil, nil, http.StatusOK
+}
+
+// CheckIfCurrentUserCanModifyCDNWithID checks if the current user has the
lock on the cdn (identified by ID) that the requested operation is to be
performed on.
+// This will succeed if the either there is no lock by any user on the CDN, or
if the current user has the lock on the CDN.
+func CheckIfCurrentUserCanModifyCDNWithID(tx *sql.Tx, cdnID int64, user
string) (error, error, int) {
+ cdnName, ok, err := GetCDNNameFromID(tx, cdnID)
+ if err != nil {
+ return nil, err, http.StatusInternalServerError
+ } else if !ok {
+ return errors.New("CDN not found"), nil, http.StatusNotFound
+ }
+ query := `SELECT username, soft FROM cdn_lock WHERE cdn=$1`
Review comment:
Now that we have the `cdnName` here, can we just call
`CheckIfCurrentUserCanModifyCDN`?
##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -1257,3 +1342,82 @@ func CheckTopologyOrgServerCGInDSCG(tx *sql.Tx, cdnIds
[]int, dsTopology string,
}
return nil, nil, http.StatusOK
}
+
+// GetCDNNameFromDSID returns the associated CDN name with the delivery
service ID that is provided.
+func GetCDNNameFromDSID(tx *sql.Tx, dsID int) (string, error) {
+ if dsID == -1 {
+ return "", nil
+ }
+ cdn := ""
+ if err := tx.QueryRow(`SELECT name FROM cdn WHERE id = (SELECT cdn_id
FROM deliveryservice WHERE id = $1)`, dsID).Scan(&cdn); err != nil {
+ return cdn, fmt.Errorf("querying CDN for deliveryservice with
ID '%v': %v", dsID, err)
+ }
+ return cdn, nil
+}
+
+// GetCDNNameFromDSXMLID returns the associated CDN name with the delivery
service XML ID that is provided.
+func GetCDNNameFromDSXMLID(tx *sql.Tx, xmlID string) (string, error) {
Review comment:
This is really similar to `GetDSIDAndCDNFromName` except that it also
returns the DS ID. Could that be reused instead and just ignore the DS ID if we
don't need it?
##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -1257,3 +1342,82 @@ func CheckTopologyOrgServerCGInDSCG(tx *sql.Tx, cdnIds
[]int, dsTopology string,
}
return nil, nil, http.StatusOK
}
+
+// GetCDNNameFromDSID returns the associated CDN name with the delivery
service ID that is provided.
+func GetCDNNameFromDSID(tx *sql.Tx, dsID int) (string, error) {
Review comment:
This is really similar to `GetDSNameAndCDNFromID` except that it also
returns the DS XMLID. Could that be reused instead and just ignore the XMLID if
we don't need it?
##########
File path: traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go
##########
@@ -611,6 +611,11 @@ func (cg *TOCacheGroup) Update(h http.Header) (error,
error, int) {
return userErr, sysErr, errCode
}
+ // CheckIfCurrentUserCanModifyCachegroup
Review comment:
this comment seems unnecessary
##########
File path: traffic_ops/traffic_ops_golang/cachegroup/cachegroups.go
##########
@@ -716,6 +721,11 @@ func (cg *TOCacheGroup) Delete() (error, error, int) {
return nil, errors.New("cachegroup delete: getting coord: " +
err.Error()), http.StatusInternalServerError
}
+ // CheckIfCurrentUserCanModifyCachegroup
Review comment:
this comment seems unnecessary
##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -1257,3 +1342,82 @@ func CheckTopologyOrgServerCGInDSCG(tx *sql.Tx, cdnIds
[]int, dsTopology string,
}
return nil, nil, http.StatusOK
}
+
+// GetCDNNameFromDSID returns the associated CDN name with the delivery
service ID that is provided.
+func GetCDNNameFromDSID(tx *sql.Tx, dsID int) (string, error) {
+ if dsID == -1 {
+ return "", nil
+ }
+ cdn := ""
+ if err := tx.QueryRow(`SELECT name FROM cdn WHERE id = (SELECT cdn_id
FROM deliveryservice WHERE id = $1)`, dsID).Scan(&cdn); err != nil {
+ return cdn, fmt.Errorf("querying CDN for deliveryservice with
ID '%v': %v", dsID, err)
+ }
+ return cdn, nil
+}
+
+// GetCDNNameFromDSXMLID returns the associated CDN name with the delivery
service XML ID that is provided.
+func GetCDNNameFromDSXMLID(tx *sql.Tx, xmlID string) (string, error) {
+ cdn := ""
+ if err := tx.QueryRow(`SELECT name FROM cdn WHERE id = (SELECT cdn_id
FROM deliveryservice WHERE xml_id = $1)`, xmlID).Scan(&cdn); err != nil {
+ return cdn, fmt.Errorf("querying CDN for deliveryservice with
XML ID '%v': %v", xmlID, err)
+ }
+ return cdn, nil
+}
+
+// GetCDNNameFromProfileID returns the cdn name for the provided profile ID.
+func GetCDNNameFromProfileID(tx *sql.Tx, id int) (tc.CDNName, error) {
+ name := ""
+ if err := tx.QueryRow(`SELECT name FROM cdn WHERE id = (SELECT cdn FROM
profile WHERE id = $1)`, id).Scan(&name); err != nil {
+ return "", errors.New("querying CDN name from profile ID: " +
err.Error())
+ }
+ return tc.CDNName(name), nil
+}
+
+// GetCDNNameFromProfileName returns the cdn name for the provided profile
name.
+func GetCDNNameFromProfileName(tx *sql.Tx, profileName string) (tc.CDNName,
error) {
+ name := ""
+ if err := tx.QueryRow(`SELECT name FROM cdn WHERE id = (SELECT cdn FROM
profile WHERE name = $1)`, profileName).Scan(&name); err != nil {
+ return "", errors.New("querying CDN name from profile name: " +
err.Error())
+ }
+ return tc.CDNName(name), nil
+}
+
+// GetServerIDsFromCachegroupNames returns a list of servers IDs for a list of
cachegroup IDs.
+func GetServerIDsFromCachegroupNames(tx *sql.Tx, cgID []string) ([]int64,
error) {
+ var serverIDs []int64
+ var serverID int64
+ query := `SELECT server.id FROM server JOIN cachegroup cg ON cg.id =
server.cachegroup where cg.name = ANY($1)`
+ rows, err := tx.Query(query, pq.Array(cgID))
+ if err != nil {
+ return serverIDs, errors.New("getting server IDs from
cachegroup names : " + err.Error())
+ }
+ defer rows.Close()
Review comment:
we could call `log.Close(rows, "some context")` here to log an error if
one occurs
##########
File path: traffic_ops/traffic_ops_golang/cachegroup/dspost.go
##########
@@ -84,6 +84,19 @@ func DSPostHandlerV40(w http.ResponseWriter, r
*http.Request) {
return
}
+ for _, dsID := range req.DeliveryServices {
+ cdn, err := dbhelpers.GetCDNNameFromDSID(inf.Tx.Tx, dsID)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx,
http.StatusInternalServerError, nil, errors.New("cachegroup deliveryservice
update: getting CDN from DS ID "+err.Error()))
+ return
+ }
+ // CheckIfCurrentUserCanModifyCDN
Review comment:
this comment seems unnecessary
##########
File path: traffic_ops/traffic_ops_golang/cachegroupparameter/parameters.go
##########
@@ -311,6 +319,12 @@ func AddCacheGroupParameters(w http.ResponseWriter, r
*http.Request) {
}
for _, p := range params {
+ // CheckIfCurrentUserCanModifyCachegroup
Review comment:
this comment seems unnecessary
##########
File path: traffic_ops/traffic_ops_golang/cachegroupparameter/parameters.go
##########
@@ -207,6 +207,11 @@ func (cgparam *TOCacheGroupParameter) Delete() (error,
error, int) {
return fmt.Errorf("parameter %v does not exist", *cgparam.ID),
nil, http.StatusNotFound
}
+ // CheckIfCurrentUserCanModifyCachegroup
Review comment:
this comment seems unnecessary
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/acme_renew.go
##########
@@ -45,6 +46,10 @@ func RenewAcmeCertificate(w http.ResponseWriter, r
*http.Request) {
return
}
defer inf.Close()
+ if inf.User == nil {
Review comment:
nit: these `inf.User == nil` checks are unnecessary because
`api.NewInfo` would return an error if it was unable to get a user, and we
always check that error
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/acme.go
##########
@@ -265,6 +278,14 @@ func GenerateLetsEncryptCertificates(w
http.ResponseWriter, r *http.Request) {
return
}
+ // CheckIfCurrentUserCanModifyCDN
Review comment:
this comment seems unnecessary
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -1167,6 +1185,26 @@ func (ds *TODeliveryService) Delete() (error, error,
int) {
}
ds.XMLID = &xmlID
+ if ds.CDNID != nil {
+ userErr, sysErr, errCode :=
dbhelpers.CheckIfCurrentUserCanModifyCDNWithID(ds.APIInfo().Tx.Tx,
int64(*ds.CDNID), ds.APIInfo().User.UserName)
+ if userErr != nil || sysErr != nil {
+ return userErr, sysErr, errCode
+ }
+ } else if ds.CDNName != nil {
+ userErr, sysErr, errCode :=
dbhelpers.CheckIfCurrentUserCanModifyCDN(ds.APIInfo().Tx.Tx, *ds.CDNName,
ds.APIInfo().User.UserName)
+ if userErr != nil || sysErr != nil {
+ return userErr, sysErr, errCode
+ }
+ } else {
+ cdnName, err := dbhelpers.GetCDNNameFromDSID(ds.ReqInfo.Tx.Tx,
*ds.ID)
+ if err != nil {
+ return fmt.Errorf("couldn't get cdn name for DS: %v",
err), nil, http.StatusBadRequest
Review comment:
I think this would be a `sysErr` instead of a `userErr` since at this
point we know the DS exists so the error would most likely be a system error
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go
##########
@@ -729,6 +736,17 @@ func UpdateV40(w http.ResponseWriter, r *http.Request) {
return
}
ds.ID = &id
+ cdn, err := dbhelpers.GetCDNNameFromDSID(inf.Tx.Tx, id)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError,
nil, errors.New("deliveryservice update: getting CDN from DS ID "+err.Error()))
+ return
+ }
+ // CheckIfCurrentUserCanModifyCDN
Review comment:
this comment seems unnecessary
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/keys.go
##########
@@ -76,7 +81,17 @@ func AddSSLKeys(w http.ResponseWriter, r *http.Request) {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusNotFound,
errors.New("no DS with name "+*req.DeliveryService), nil)
return
}
-
+ cdn, err := dbhelpers.GetCDNNameFromDSID(inf.Tx.Tx, dsID)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError,
nil, errors.New("deliveryservice.AddSSLKeys: getting CDN from DS ID
"+err.Error()))
+ return
+ }
+ // CheckIfCurrentUserCanModifyCDN
Review comment:
this comment seems unnecessary
##########
File path: traffic_ops/traffic_ops_golang/servercheck/servercheck.go
##########
@@ -131,6 +131,16 @@ func CreateUpdateServercheck(w http.ResponseWriter, r
*http.Request) {
return
}
+ cdnName, err := dbhelpers.GetCDNNameFromServerID(inf.Tx.Tx, int64(id))
Review comment:
`servercheck` probably doesn't need to be lock-safe since it's primarily
just for monitoring and doesn't really affect CDN operation
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/autorenewcerts.go
##########
@@ -100,6 +105,17 @@ func renewCertificates(w http.ResponseWriter, r
*http.Request, deprecated bool)
api.HandleErr(w, r, inf.Tx.Tx,
http.StatusInternalServerError, nil, err)
return
}
+ cdn, err := dbhelpers.GetCDNNameFromDSXMLID(inf.Tx.Tx, ds.XmlId)
Review comment:
I'm pretty sure this will cause an error because there are still unread
rows in the current transaction that we're using here. We have to fully read
the rows before being able to reuse the same transaction for another query.
Also, similar to my earlier comment, we wouldn't want to run a DB query for
every single DS and cert -- rather, we should run a query to get all the
distinct CDN names of the DSes here then pass that result to another function
which runs a query to check all the CDNs at once.
##########
File path: traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go
##########
@@ -1257,3 +1342,82 @@ func CheckTopologyOrgServerCGInDSCG(tx *sql.Tx, cdnIds
[]int, dsTopology string,
}
return nil, nil, http.StatusOK
}
+
+// GetCDNNameFromDSID returns the associated CDN name with the delivery
service ID that is provided.
+func GetCDNNameFromDSID(tx *sql.Tx, dsID int) (string, error) {
+ if dsID == -1 {
+ return "", nil
+ }
+ cdn := ""
+ if err := tx.QueryRow(`SELECT name FROM cdn WHERE id = (SELECT cdn_id
FROM deliveryservice WHERE id = $1)`, dsID).Scan(&cdn); err != nil {
+ return cdn, fmt.Errorf("querying CDN for deliveryservice with
ID '%v': %v", dsID, err)
+ }
+ return cdn, nil
+}
+
+// GetCDNNameFromDSXMLID returns the associated CDN name with the delivery
service XML ID that is provided.
+func GetCDNNameFromDSXMLID(tx *sql.Tx, xmlID string) (string, error) {
+ cdn := ""
+ if err := tx.QueryRow(`SELECT name FROM cdn WHERE id = (SELECT cdn_id
FROM deliveryservice WHERE xml_id = $1)`, xmlID).Scan(&cdn); err != nil {
+ return cdn, fmt.Errorf("querying CDN for deliveryservice with
XML ID '%v': %v", xmlID, err)
+ }
+ return cdn, nil
+}
+
+// GetCDNNameFromProfileID returns the cdn name for the provided profile ID.
+func GetCDNNameFromProfileID(tx *sql.Tx, id int) (tc.CDNName, error) {
+ name := ""
+ if err := tx.QueryRow(`SELECT name FROM cdn WHERE id = (SELECT cdn FROM
profile WHERE id = $1)`, id).Scan(&name); err != nil {
+ return "", errors.New("querying CDN name from profile ID: " +
err.Error())
+ }
+ return tc.CDNName(name), nil
+}
+
+// GetCDNNameFromProfileName returns the cdn name for the provided profile
name.
+func GetCDNNameFromProfileName(tx *sql.Tx, profileName string) (tc.CDNName,
error) {
+ name := ""
+ if err := tx.QueryRow(`SELECT name FROM cdn WHERE id = (SELECT cdn FROM
profile WHERE name = $1)`, profileName).Scan(&name); err != nil {
+ return "", errors.New("querying CDN name from profile name: " +
err.Error())
+ }
+ return tc.CDNName(name), nil
+}
+
+// GetServerIDsFromCachegroupNames returns a list of servers IDs for a list of
cachegroup IDs.
+func GetServerIDsFromCachegroupNames(tx *sql.Tx, cgID []string) ([]int64,
error) {
+ var serverIDs []int64
+ var serverID int64
+ query := `SELECT server.id FROM server JOIN cachegroup cg ON cg.id =
server.cachegroup where cg.name = ANY($1)`
+ rows, err := tx.Query(query, pq.Array(cgID))
+ if err != nil {
+ return serverIDs, errors.New("getting server IDs from
cachegroup names : " + err.Error())
+ }
+ defer rows.Close()
+ for rows.Next() {
+ err = rows.Scan(&serverID)
+ if err != nil {
+ return serverIDs, errors.New("scanning server ID : " +
err.Error())
+ }
+ serverIDs = append(serverIDs, serverID)
+ }
+ return serverIDs, nil
+}
+
+// GetCDNNamesFromServerIds returns a list of cdn names for a list of server
IDs.
+func GetCDNNamesFromServerIds(tx *sql.Tx, serverIds []int64) ([]string, error)
{
+ var cdns []string
+ cdn := ""
+ query := `SELECT DISTINCT(name) FROM cdn JOIN server ON cdn.id =
server.cdn_id WHERE server.id = ANY($1)`
+ rows, err := tx.Query(query, pq.Array(serverIds))
+ if err != nil {
+ return cdns, errors.New("getting cdn name for server : " +
err.Error())
+ }
+ defer rows.Close()
Review comment:
we could call `log.Close(rows, "some context")` here to log an error if
one occurs
##########
File path: traffic_ops/traffic_ops_golang/cachegroupparameter/parameters.go
##########
@@ -311,6 +319,12 @@ func AddCacheGroupParameters(w http.ResponseWriter, r
*http.Request) {
}
for _, p := range params {
+ // CheckIfCurrentUserCanModifyCachegroup
+ userErr, sysErr, errCode :=
dbhelpers.CheckIfCurrentUserCanModifyCachegroup(inf.Tx.Tx, *p.CacheGroup,
inf.User.UserName)
Review comment:
similar to my earlier comment, we should try to make one query to check
all the given cachegroups at once, rather than looping over each parameter in
the request
##########
File path: traffic_ops/traffic_ops_golang/cachegroup/dspost.go
##########
@@ -84,6 +84,19 @@ func DSPostHandlerV40(w http.ResponseWriter, r
*http.Request) {
return
}
+ for _, dsID := range req.DeliveryServices {
+ cdn, err := dbhelpers.GetCDNNameFromDSID(inf.Tx.Tx, dsID)
Review comment:
We should really avoid making DB queries in a loop like this because it
scales linearly with the number of delivery services given in the request. Can
we make one query to get all the distinct DS CDN names, then run a 2nd query to
check all the CDNs at once?
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/acme_renew.go
##########
@@ -56,10 +61,22 @@ func RenewAcmeCertificate(w http.ResponseWriter, r
*http.Request) {
return
}
+ cdn, err := dbhelpers.GetCDNNameFromDSXMLID(inf.Tx.Tx, xmlID)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx, http.StatusInternalServerError,
nil, errors.New("renew acme certificate: getting CDN from DS XML ID
"+err.Error()))
+ return
+ }
+ // CheckIfCurrentUserCanModifyCDN
Review comment:
comment seems unnecessary
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/acme.go
##########
@@ -205,10 +209,19 @@ func GenerateAcmeCertificates(w http.ResponseWriter, r
*http.Request) {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest,
errors.New("delivery service not in cdn"), nil)
return
}
+ // CheckIfCurrentUserCanModifyCDN
Review comment:
this comment seems unnecessary
##########
File path: traffic_ops/traffic_ops_golang/federations/ds.go
##########
@@ -63,6 +63,18 @@ func PostDSes(w http.ResponseWriter, r *http.Request) {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest,
errors.New("A federation must have at least one delivery service assigned"),
nil)
return
}
+ for _, dsID := range post.DSIDs {
+ cdnName, err := dbhelpers.GetCDNNameFromDSID(inf.Tx.Tx,
dsID)
Review comment:
ditto earlier comments about running DB queries in a loop
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/autorenewcerts.go
##########
@@ -100,6 +105,17 @@ func renewCertificates(w http.ResponseWriter, r
*http.Request, deprecated bool)
api.HandleErr(w, r, inf.Tx.Tx,
http.StatusInternalServerError, nil, err)
return
}
+ cdn, err := dbhelpers.GetCDNNameFromDSXMLID(inf.Tx.Tx, ds.XmlId)
+ if err != nil {
+ api.HandleErr(w, r, inf.Tx.Tx,
http.StatusInternalServerError, nil, errors.New("renew acme certificate:
getting CDN from DS XML ID "+err.Error()))
+ return
+ }
+ // CheckIfCurrentUserCanModifyCDN
Review comment:
this comment seems unnecessary
##########
File path:
traffic_ops/traffic_ops_golang/profileparameter/postparameterprofile.go
##########
@@ -45,6 +46,20 @@ func PostParamProfile(w http.ResponseWriter, r
*http.Request) {
api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest,
errors.New("parse error: "+err.Error()), nil)
return
}
+ if paramProfile.ProfileIDs != nil {
+ for _, profileID := range *paramProfile.ProfileIDs {
+ cdnName, err :=
dbhelpers.GetCDNNameFromProfileID(inf.Tx.Tx, int(profileID))
Review comment:
ditto earlier comments about running DB queries in a loop
##########
File path: traffic_ops/traffic_ops_golang/topology/topologies.go
##########
@@ -806,3 +821,22 @@ func selectMaxLastUpdatedQuery(where string) string {
` UNION ALL
select max(last_updated) as ti from last_deleted l where
l.table_name='topology') as res`
}
+
+func (topology *TOTopology) checkIfTopologyCanBeAlteredByCurrentUser() (error,
error, int) {
+ cachegroups := topology.getCachegroupNames()
+ serverIDs, err :=
dbhelpers.GetServerIDsFromCachegroupNames(topology.ReqInfo.Tx.Tx, cachegroups)
+ if err != nil {
+ return nil, err, http.StatusInternalServerError
+ }
+ cdns, err := dbhelpers.GetCDNNamesFromServerIds(topology.ReqInfo.Tx.Tx,
serverIDs)
+ if err != nil {
+ return nil, err, http.StatusInternalServerError
+ }
+ for _, cdn := range cdns {
+ userErr, sysErr, statusCode :=
dbhelpers.CheckIfCurrentUserCanModifyCDN(topology.ReqInfo.Tx.Tx, cdn,
topology.ReqInfo.User.UserName)
Review comment:
ditto earlier comments about making DB queries in a loop
##########
File path: traffic_ops/traffic_ops_golang/deliveryservice/keys.go
##########
@@ -29,6 +29,7 @@ import (
"encoding/pem"
"errors"
"fmt"
+
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
Review comment:
nit: I just noticed that this import is out of order in most of these
changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
