Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix consistency of named hydrate call usage #650

Merged
merged 3 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions plugin/hydrate_call.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
type hydrateCall struct {
namedHydrateFunc
// the dependencies expressed using function name
Depends []string
Depends []namedHydrateFunc
Config *HydrateConfig

queryData *QueryData
Expand All @@ -31,7 +31,7 @@ func newHydrateCall(config *HydrateConfig, d *QueryData) (*hydrateCall, error) {
res.namedHydrateFunc = newNamedHydrateFunc(config.Func)

for _, f := range config.Depends {
res.Depends = append(res.Depends, helpers.GetFunctionName(f))
res.Depends = append(res.Depends, newNamedHydrateFunc(f))
}

return res, nil
Expand Down Expand Up @@ -75,7 +75,7 @@ func (h *hydrateCall) initialiseRateLimiter() error {
func (h *hydrateCall) canStart(rowData *rowData) bool {
// check whether all hydrate functions we depend on have saved their results
for _, dep := range h.Depends {
if !helpers.StringSliceContains(rowData.getHydrateKeys(), dep) {
if !helpers.StringSliceContains(rowData.getHydrateKeys(), dep.Name) {
return false
}
}
Expand Down
17 changes: 11 additions & 6 deletions plugin/hydrate_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"log"
"strings"

"github.com/turbot/go-kit/helpers"
"github.com/turbot/steampipe-plugin-sdk/v5/rate_limiter"
)

Expand Down Expand Up @@ -114,19 +113,21 @@ type HydrateConfig struct {

// Deprecated: use IgnoreConfig
ShouldIgnoreError ErrorPredicate

namedFunc namedHydrateFunc
}

func (c *HydrateConfig) String() string {
var dependsStrings = make([]string, len(c.Depends))
for i, dep := range c.Depends {
dependsStrings[i] = helpers.GetFunctionName(dep)
dependsStrings[i] = newNamedHydrateFunc(dep).Name
}
str := fmt.Sprintf(`Func: %s
RetryConfig: %s
IgnoreConfig: %s
Depends: %s
ScopeValues: %s`,
helpers.GetFunctionName(c.Func),
c.namedFunc.Name,
c.RetryConfig,
c.IgnoreConfig,
strings.Join(dependsStrings, ","),
Expand All @@ -136,7 +137,9 @@ ScopeValues: %s`,
}

func (c *HydrateConfig) initialise(table *Table) {
log.Printf("[TRACE] HydrateConfig.initialise func %s, table %s", helpers.GetFunctionName(c.Func), table.Name)
c.namedFunc = newNamedHydrateFunc(c.Func)

log.Printf("[TRACE] HydrateConfig.initialise func %s, table %s", c.namedFunc.Name, table.Name)

// create RetryConfig if needed
if c.RetryConfig == nil {
Expand All @@ -159,8 +162,10 @@ func (c *HydrateConfig) initialise(table *Table) {
}

// default ignore and retry configs to table defaults
c.RetryConfig.DefaultTo(table.DefaultRetryConfig)
c.IgnoreConfig.DefaultTo(table.DefaultIgnoreConfig)
if table != nil {
c.RetryConfig.DefaultTo(table.DefaultRetryConfig)
c.IgnoreConfig.DefaultTo(table.DefaultIgnoreConfig)
}

log.Printf("[TRACE] HydrateConfig.initialise complete: RetryConfig: %s, IgnoreConfig: %s", c.RetryConfig.String(), c.IgnoreConfig.String())
}
Expand Down
7 changes: 3 additions & 4 deletions plugin/list_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package plugin
import (
"fmt"
"github.com/gertd/go-pluralize"
"github.com/turbot/go-kit/helpers"
"log"
)

Expand Down Expand Up @@ -108,9 +107,9 @@ func (c *ListConfig) Validate(table *Table) []string {
}

// ensure that if there is an explicit hydrate config for the list hydrate, it does not declare dependencies
listHydrateName := helpers.GetFunctionName(table.List.Hydrate)
listHydrateName := table.List.namedHydrateFunc.Name
for _, h := range table.HydrateConfig {
if helpers.GetFunctionName(h.Func) == listHydrateName {
if h.namedFunc.Name == listHydrateName {
if len(h.Depends) > 0 {
validationErrors = append(validationErrors, fmt.Sprintf("table '%s' List hydrate function '%s' defines dependencies in its `HydrateConfig`", table.Name, listHydrateName))
}
Expand All @@ -119,7 +118,7 @@ func (c *ListConfig) Validate(table *Table) []string {
}
// ensure there is no hydrate dependency declared for the list hydrate
for _, h := range table.HydrateDependencies {
if helpers.GetFunctionName(h.Func) == listHydrateName {
if newNamedHydrateFunc(h.Func).Name == listHydrateName {
numDeps := len(h.Depends)
validationErrors = append(validationErrors, fmt.Sprintf("table '%s' List hydrate function '%s' has %d %s - List hydrate functions cannot have dependencies",
table.Name,
Expand Down
4 changes: 3 additions & 1 deletion plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,9 @@ func (p *Plugin) buildHydrateConfigMap() {
// as we are converting into a pointer, we cannot use the array value direct from the range as
// this was causing incorrect values - go must be reusing memory addresses for successive items
h := &p.HydrateConfig[i]
funcName := helpers.GetFunctionName(h.Func)
// initialise before adding to map
h.initialise(nil)
funcName := h.namedFunc.Name
p.hydrateConfigMap[funcName] = h
}
}
Expand Down
23 changes: 12 additions & 11 deletions plugin/query_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,11 +393,10 @@

// what is the name of the fetch call (i.e. the get/list call)
fetchFunc := t.getFetchFunc(fetchType)
fetchCallName := helpers.GetFunctionName(fetchFunc)

// initialise hydrateColumnMap
d.hydrateColumnMap = make(map[string][]string)
requiredCallBuilder := newRequiredHydrateCallBuilder(d, fetchCallName)
requiredCallBuilder := newRequiredHydrateCallBuilder(d, fetchFunc.Name)

// populate a map keyed by function name to ensure we only store each hydrate function once
for _, column := range t.Columns {
Expand All @@ -413,14 +412,16 @@
// so there is NO hydrate call registered for the column
// the column is provided by the fetch call
// do not add to map of hydrate functions as the fetch call will always be called
hydrateFunc = fetchFunc
hydrateName = fetchCallName
hydrateFunc = fetchFunc.Func
hydrateName = fetchFunc.Name
} else {
// there is a hydrate call registered
hydrateName = helpers.GetFunctionName(hydrateFunc)
namedFunc := newNamedHydrateFunc(hydrateFunc)
hydrateName = namedFunc.Name

// if this column was requested in query, add the hydrate call to required calls
if helpers.StringSliceContains(colsUsed, column.Name) {
requiredCallBuilder.Add(hydrateFunc, d.connectionCallId)
requiredCallBuilder.Add(namedFunc, d.connectionCallId)

Check failure on line 424 in plugin/query_data.go

View workflow job for this annotation

GitHub Actions / Build

Error return value of `requiredCallBuilder.Add` is not checked (errcheck)
}
}

Expand All @@ -436,7 +437,7 @@
// build list of all columns returned by the fetch call and required hydrate calls
func (d *QueryData) populateColumns() {
// add columns returned by fetch call
fetchName := helpers.GetFunctionName(d.Table.getFetchFunc(d.FetchType))
fetchName := d.Table.getFetchFunc(d.FetchType).Name
d.addColumnsForHydrate(fetchName)

// add columns returned by required hydrate calls
Expand Down Expand Up @@ -536,13 +537,13 @@
if d.Table.List == nil {
return false
}
listFunction := helpers.GetFunctionName(d.Table.List.Hydrate)
listParentFunction := helpers.GetFunctionName(d.Table.List.ParentHydrate)
listFunction := d.Table.List.namedHydrateFunc.Name
listParentFunction := d.Table.List.namedParentHydrateFunc.Name
if callingFunction != listFunction && callingFunction != listParentFunction {
// if the calling function is NOT one of the other registered hydrate functions,
//it must be an anonymous function so let it go
for _, c := range d.Table.Columns {
if c.Hydrate != nil && helpers.GetFunctionName(c.Hydrate) == callingFunction {
if c.Hydrate != nil && newNamedHydrateFunc(c.Hydrate).Name == callingFunction {
return false
}
}
Expand Down Expand Up @@ -650,7 +651,7 @@
// set the parent item on the row data
rd.parentItem = d.parentItem
// NOTE: add the item as the hydrate data for the list call
rd.set(helpers.GetFunctionName(d.Table.List.Hydrate), item)
rd.set(d.Table.List.namedHydrateFunc.Name, item)

Check failure on line 654 in plugin/query_data.go

View workflow job for this annotation

GitHub Actions / Build

Error return value of `rd.set` is not checked (errcheck)

d.rowDataChan <- rd
}
Expand Down
15 changes: 7 additions & 8 deletions plugin/query_data_rate_limiters.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package plugin

import (
"context"
"github.com/turbot/go-kit/helpers"
"github.com/turbot/steampipe-plugin-sdk/v5/grpc"
"github.com/turbot/steampipe-plugin-sdk/v5/plugin/quals"
"github.com/turbot/steampipe-plugin-sdk/v5/rate_limiter"
Expand Down Expand Up @@ -97,7 +96,7 @@ func (d *QueryData) resolveGetRateLimiters() error {
// NOTE: RateLimit cannot be nil as it is initialized to an empty struct if needed
getLimiter, err := d.plugin.getHydrateCallRateLimiter(d.Table.Get.Tags, d)
if err != nil {
log.Printf("[WARN] get call %s getHydrateCallRateLimiter failed: %s (%s)", helpers.GetFunctionName(d.Table.Get.Hydrate), err.Error(), d.connectionCallId)
log.Printf("[WARN] get call %s getHydrateCallRateLimiter failed: %s (%s)", d.Table.Get.named.Name, err.Error(), d.connectionCallId)
return err
}

Expand All @@ -112,7 +111,7 @@ func (d *QueryData) resolveParentChildRateLimiters() error {
// resolve the parent hydrate rate limiter
parentRateLimiter, err := d.plugin.getHydrateCallRateLimiter(d.Table.List.ParentTags, d)
if err != nil {
log.Printf("[WARN] resolveParentChildRateLimiters: %s: getHydrateCallRateLimiter failed: %s (%s)", helpers.GetFunctionName(d.Table.List.ParentHydrate), err.Error(), d.connectionCallId)
log.Printf("[WARN] resolveParentChildRateLimiters: %s: getHydrateCallRateLimiter failed: %s (%s)", d.Table.List.namedParentHydrateFunc.Name, err.Error(), d.connectionCallId)
return err
}
// assign the parent rate limiter to d.fetchLimiters
Expand All @@ -121,7 +120,7 @@ func (d *QueryData) resolveParentChildRateLimiters() error {
// resolve the child hydrate rate limiter
childRateLimiter, err := d.plugin.getHydrateCallRateLimiter(d.Table.List.Tags, d)
if err != nil {
log.Printf("[WARN] resolveParentChildRateLimiters: %s: getHydrateCallRateLimiter failed: %s (%s)", helpers.GetFunctionName(d.Table.List.Hydrate), err.Error(), d.connectionCallId)
log.Printf("[WARN] resolveParentChildRateLimiters: %s: getHydrateCallRateLimiter failed: %s (%s)", d.Table.List.namedHydrateFunc.Name, err.Error(), d.connectionCallId)
return err
}
d.fetchLimiters.childListRateLimiter = childRateLimiter
Expand All @@ -133,7 +132,7 @@ func (d *QueryData) resolveListRateLimiters() error {
// NOTE: RateLimit cannot be nil as it is initialized to an empty struct if needed
listLimiter, err := d.plugin.getHydrateCallRateLimiter(d.Table.List.Tags, d)
if err != nil {
log.Printf("[WARN] get call %s getHydrateCallRateLimiter failed: %s (%s)", helpers.GetFunctionName(d.Table.Get.Hydrate), err.Error(), d.connectionCallId)
log.Printf("[WARN] get call %s getHydrateCallRateLimiter failed: %s (%s)", d.Table.Get.named.Name, err.Error(), d.connectionCallId)
return err
}
d.fetchLimiters.rateLimiter = listLimiter
Expand All @@ -142,7 +141,7 @@ func (d *QueryData) resolveListRateLimiters() error {

func (d *QueryData) setListLimiterMetadata(fetchDelay time.Duration) {
fetchMetadata := &hydrateMetadata{
FuncName: helpers.GetFunctionName(d.listHydrate),
FuncName: d.listHydrate.Name,
RateLimiters: d.fetchLimiters.rateLimiter.LimiterNames(),
ScopeValues: d.fetchLimiters.rateLimiter.ScopeValues,
DelayMs: fetchDelay.Milliseconds(),
Expand All @@ -153,7 +152,7 @@ func (d *QueryData) setListLimiterMetadata(fetchDelay time.Duration) {
} else {
d.fetchMetadata = &hydrateMetadata{
Type: string(fetchTypeList),
FuncName: helpers.GetFunctionName(d.childHydrate),
FuncName: d.childHydrate.Name,
RateLimiters: d.fetchLimiters.childListRateLimiter.LimiterNames(),
ScopeValues: d.fetchLimiters.childListRateLimiter.ScopeValues,
}
Expand All @@ -165,7 +164,7 @@ func (d *QueryData) setListLimiterMetadata(fetchDelay time.Duration) {
func (d *QueryData) setGetLimiterMetadata(fetchDelay time.Duration) {
d.fetchMetadata = &hydrateMetadata{
Type: string(fetchTypeGet),
FuncName: helpers.GetFunctionName(d.Table.Get.Hydrate),
FuncName: d.Table.Get.named.Name,
RateLimiters: d.fetchLimiters.rateLimiter.LimiterNames(),
ScopeValues: d.fetchLimiters.rateLimiter.ScopeValues,
DelayMs: fetchDelay.Milliseconds(),
Expand Down
10 changes: 5 additions & 5 deletions plugin/required_hydrate_calls.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package plugin

import (
"github.com/turbot/go-kit/helpers"
"log"
)

Expand All @@ -20,8 +19,8 @@ func newRequiredHydrateCallBuilder(d *QueryData, fetchCallName string) *required
}
}

func (c requiredHydrateCallBuilder) Add(hydrateFunc HydrateFunc, callId string) error {
hydrateName := helpers.GetFunctionName(hydrateFunc)
func (c requiredHydrateCallBuilder) Add(hydrateFunc namedHydrateFunc, callId string) error {
hydrateName := hydrateFunc.Name

// if the resolved hydrate call is NOT the same as the fetch call, add to the map of hydrate functions to call
if hydrateName != c.fetchCallName {
Expand All @@ -41,8 +40,9 @@ func (c requiredHydrateCallBuilder) Add(hydrateFunc HydrateFunc, callId string)

// now add dependencies (we have already checked for circular dependencies so recursion is fine
for _, dep := range config.Depends {
if err := c.Add(dep, callId); err != nil {
log.Printf("[WARN] failed to add a hydrate call for %s, which is a dependency of %s: %s", helpers.GetFunctionName(dep), hydrateName, err.Error())
namedDep := newNamedHydrateFunc(dep)
if err := c.Add(namedDep, callId); err != nil {
log.Printf("[WARN] failed to add a hydrate call for %s, which is a dependency of %s: %s", namedDep.Name, hydrateName, err.Error())
return err
}
}
Expand Down
30 changes: 10 additions & 20 deletions plugin/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"github.com/turbot/steampipe-plugin-sdk/v5/rate_limiter"
"log"

"github.com/turbot/go-kit/helpers"
"github.com/turbot/steampipe-plugin-sdk/v5/plugin/transform"
)

Expand Down Expand Up @@ -132,7 +131,7 @@ func (t *Table) initialise(p *Plugin) {
// declared for specific columns which do not have config defined
// build a map of all hydrate functions, with empty config if needed
// NOTE: this map also includes information from the legacy HydrateDependencies property
t.initialiseHydrateConfigs()
t.buildHydrateConfigMap()

t.setColumnNameMap()

Expand All @@ -148,17 +147,6 @@ func (t *Table) setColumnNameMap() {
}
}

// build map of all hydrate configs, and initialise them
func (t *Table) initialiseHydrateConfigs() {
// first build a map of all hydrate functions
t.buildHydrateConfigMap()

// initialise all hydrate configs in map
for _, h := range t.hydrateConfigMap {
h.initialise(t)
}
}

// build map of all hydrate configs, including those specified in the legacy HydrateDependencies,
// and those mentioned only in column config
func (t *Table) buildHydrateConfigMap() {
Expand All @@ -167,12 +155,14 @@ func (t *Table) buildHydrateConfigMap() {
// as we are converting into a pointer, we cannot use the array value direct from the range as
// this was causing incorrect values - go must be reusing memory addresses for successive items
h := &t.HydrateConfig[i]
funcName := helpers.GetFunctionName(h.Func)
t.hydrateConfigMap[funcName] = h

// initialise before adding to map
h.initialise(t)
t.hydrateConfigMap[h.namedFunc.Name] = h
}
// add in hydrate config for all hydrate dependencies declared using legacy property HydrateDependencies
for _, d := range t.HydrateDependencies {
hydrateName := helpers.GetFunctionName(d.Func)
hydrateName := newNamedHydrateFunc(d.Func).Name
// if there is already a hydrate config, do nothing here
// (this is a validation error that will be picked up by the validation check later)
if _, ok := t.hydrateConfigMap[hydrateName]; !ok {
Expand All @@ -181,7 +171,7 @@ func (t *Table) buildHydrateConfigMap() {
}
// NOTE: the get config may be used as a column hydrate function so add this into the map
if get := t.Get; get != nil {
hydrateName := helpers.GetFunctionName(get.Hydrate)
hydrateName := get.named.Name
t.hydrateConfigMap[hydrateName] = &HydrateConfig{
Func: get.Hydrate,
IgnoreConfig: get.IgnoreConfig,
Expand All @@ -208,9 +198,9 @@ func (t *Table) buildHydrateConfigMap() {
}
}

func (t *Table) getFetchFunc(fetchType fetchType) HydrateFunc {
func (t *Table) getFetchFunc(fetchType fetchType) namedHydrateFunc {
if fetchType == fetchTypeList {
return t.List.Hydrate
return *t.List.namedHydrateFunc
}
return t.Get.Hydrate
return t.Get.named
}
6 changes: 3 additions & 3 deletions plugin/table_fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (t *Table) executeGetCall(ctx context.Context, queryData *QueryData) (err e
// we can now close the item chan
queryData.fetchComplete(ctx)
if r := recover(); r != nil {
err = status.Error(codes.Internal, fmt.Sprintf("get call %s failed with panic %v", helpers.GetFunctionName(t.Get.Hydrate), r))
err = status.Error(codes.Internal, fmt.Sprintf("get call %s failed with panic %v", t.Get.named.Name, r))
}
}()

Expand Down Expand Up @@ -179,7 +179,7 @@ func (t *Table) doGetForQualValues(ctx context.Context, queryData *QueryData, ke
// execute a get call for a single key column qual value
// if a matrix is defined, call for every matrix item
func (t *Table) doGet(ctx context.Context, queryData *QueryData) (err error) {
hydrateKey := helpers.GetFunctionName(t.Get.Hydrate)
hydrateKey := t.Get.named.Name
defer func() {
if p := recover(); p != nil {
err = status.Error(codes.Internal, fmt.Sprintf("table '%s': Get hydrate call %s failed with panic %v", t.Name, hydrateKey, p))
Expand Down Expand Up @@ -358,7 +358,7 @@ func (t *Table) executeListCall(ctx context.Context, queryData *QueryData) {
defer log.Printf("[TRACE] executeListCall COMPLETE (%s)", queryData.connectionCallId)
defer func() {
if r := recover(); r != nil {
queryData.streamError(status.Error(codes.Internal, fmt.Sprintf("list call %s failed with panic %v", helpers.GetFunctionName(t.List.Hydrate), r)))
queryData.streamError(status.Error(codes.Internal, fmt.Sprintf("list call %s failed with panic %v", t.List.namedHydrateFunc.Name, r)))
}
// list call will return when it has streamed all items so close rowDataChan
queryData.fetchComplete(ctx)
Expand Down
Loading
Loading