Skip to content

feat(Slash gRPC Client): Add gRPC support for Slash endpoints. #137

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 16, 2020
Merged

feat(Slash gRPC Client): Add gRPC support for Slash endpoints. #137

merged 3 commits into from
Sep 16, 2020

Conversation

all-seeing-code
Copy link
Contributor

@all-seeing-code all-seeing-code commented Sep 14, 2020 •
edited
Loading

Motivation

This PR add support for gRPC client for Slash backends.
Usage:

graphQlEndpoint := "https://winged-breath.us-west-2.aws.cloud.dgraph.io/graphql"
apiKey := ""
dg := dgo.DialSlashGraphQLEndpoint(graphQlEndpoint, apiKey)

This change is Reviewable

@all-seeing-code all-seeing-code changed the title feat(slash client): Extend dgo client with Slash endpoint handling feat(Slash gRPC Client): Add gRPC support for Slash endpoints. Sep 14, 2020
parasssh
parasssh reviewed Sep 14, 2020
View reviewed changes
client.go Outdated
token string
}

func (a *authorizationCredentials) GetRequestMetadata(ctx context.Context, uri ...string) (map[string]string, error) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need example and unit test on how this is used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can he just link to the appropriate interface that is being implemented?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is required here? I have resolved the other requirements.

gja
gja suggested changes Sep 14, 2020
View reviewed changes
Copy link
Contributor

@gja gja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code wise it looks great, but I'm curious. why you have to include dgo v1.0

gja
gja suggested changes Sep 14, 2020
View reviewed changes
martinmr
martinmr suggested changes Sep 14, 2020
View reviewed changes
Copy link
Contributor

@martinmr martinmr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @anurags92 and @parasssh)

client.go, line 45 at r1 (raw file):

	dc       []api.DgraphClient
}
type authorizationCredentials struct {

rename to authCreds or something shorter

@all-seeing-code all-seeing-code mentioned this pull request Sep 16, 2020
Merged
@all-seeing-code all-seeing-code merged commit 9ff368a into master Sep 16, 2020
@all-seeing-code all-seeing-code mentioned this pull request Sep 21, 2020
Closed
@mangalaman93 mangalaman93 deleted the anurags92/SlashEndpoint branch March 22, 2023 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@parasssh parasssh parasssh left review comments

@martinmr martinmr martinmr requested changes

@gja gja gja approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@all-seeing-code @gja @martinmr @parasssh