Refactor all Get hydrates

[japborst]

There's a lot of duplication when hydrating the data. Likely from copy-pasting over time.

The aim of this PR is to decrease that for "get" hydrates.

Refactoring the list hydrates are a bit trickier, so I started out with just the gets. Lists can be done in a separate PR.

• Refactor get hydrates into a single function
• Always log upon hydrating

[japborst]

It would be nice to have this built into the SDK, similar to d.StreamListItem.

[misraved]

Great work @japborst !!!

I had an initial pass at the changes and they look good. I will have a detailed look at it and provide feedback 👍 .

The idea of creating streamList function sounds interesting. Could you also create an issue in the SDK repo so that we could discuss more on it?

[japborst]

Thanks, @misraved. Looking forward to your comments.

Created the issue here turbot/steampipe-plugin-sdk#341.

[japborst]

@misraved I noticed I missed a few use-cases. Should be complete now 👍

[github-actions]

'This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.'

[japborst]

Not stale. Needs a review.

[japborst]

@cbruno10 what would you like to do with this PR?

[cbruno10]

Hey @japborst , sorry for the long delay. It's still on our radar to review, but we have some other items we're focusing on right now, and due to its scope, I don't want to rush a review. I'd like to leave the PR open and review some time in the next few weeks.

[github-actions]

'This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.'

[misraved]

Not stale. It will be reviewed shortly.

[github-actions]

'This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.'

[github-actions]

'This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.'

[japborst]

Not stale

[cbruno10]

Hi @japborst , sorry it's taken so long to reply to this PR!

We've been deciding on what the GitHub plugin should look like, mainly around which API version should it use for its tables, and ultimately decided that we should update all tables to use the GitHub v4 GraphQL API. With this change, we will use a different Go SDK and the code structure itself will change, e.g.,

steampipe-plugin-github/github/table_github_organization_member.go

Lines 62 to 109 in 78cff7e

	func tableGitHubOrganizationMemberList(ctx context.Context, d *plugin.QueryData, h *plugin.HydrateData) (interface{}, error) {
	client := connectV4(ctx, d)
	quals := d.EqualsQuals
	org := quals["organization"].GetStringValue()
	pageSize := 100
	limit := d.QueryContext.Limit
	if limit != nil {
	if *limit < int64(pageSize) {
	pageSize = int(*limit)
	}
	}
	variables := map[string]interface{}{
	"login": githubv4.String(org),
	"membersWithRolePageSize": githubv4.Int(pageSize),
	"membersWithRoleCursor": (*githubv4.String)(nil), // Null after argument to get first page.
	}
	for {
	err := client.Query(ctx, &query, variables)
	if err != nil {
	plugin.Logger(ctx).Error("github_organization_member", "api_error", err)
	if strings.Contains(err.Error(), "Could not resolve to an Organization with the login of") {
	return nil, nil
	}
	return nil, err
	}
	for _, member := range query.Organization.MembersWithRole.Edges {
	d.StreamListItem(ctx, member)
	// Context can be cancelled due to manual cancellation or the limit has been hit
	if d.RowsRemaining(ctx) == 0 {
	return nil, nil
	}
	}
	if !query.Organization.MembersWithRole.PageInfo.HasNextPage {
	break
	}
	variables["membersWithRoleCursor"] = githubv4.NewString(query.Organization.MembersWithRole.PageInfo.EndCursor)
	}
	return nil, nil
	}

.

The new code should in theory be lighter than it is now, as we'll be requesting the exact fields for each resource, so the number of hydrate calls should decrease overall.

At this time, we most likely will not accept this PR, as I believe the GraphQL API changes will change the codebase in a significant way, but if these changes are applicable to the updated code in some way, we're happy to re-open and discuss more in this PR (or in a new one if easier).

[japborst]

Hey, thanks for the reply!

Using the GraphQL API makes a lot of sense. I really think you can get a big performance boost out of it, by reducing the number of calls to hydrate the data.

[cbruno10]

@japborst Definitely, early tests are promising, so hoping we can get better performance so large queries and our mods will work smoother (rate limits are killer).