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

Adding auth token support #2692

Merged
merged 2 commits into from
Oct 24, 2018
Merged

Adding auth token support #2692

merged 2 commits into from
Oct 24, 2018

Conversation

gitlw
Copy link

@gitlw gitlw commented Oct 24, 2018

Performed testing using the following steps:

  1. The alpha server is started with the --auth_token option
    2.a The live loader command is run without any change of schema through the --schema option, then the --auth_token option is not required, i.e. the rdf file can still be processed successfully without the --auth_token option
    2.b The live loader command is run with a schema change through the --schema option, then the --auth_token option must be provided, otherwise the Alter gRPC call fails with an error complaining about missing the auth token

This change is Reviewable

@CLAassistant
Copy link

CLAassistant commented Oct 24, 2018

CLA assistant check
All committers have signed the CLA.

@@ -22,6 +22,7 @@ import (
"compress/gzip"
"context"
"fmt"
"google.golang.org/grpc/metadata"

Choose a reason for hiding this comment

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

File is not goimports-ed

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @manishrjain and @gitlw)


dgraph/cmd/live/run.go, line 25 at r1 (raw file):

Previously, golangcibot (Bot from GolangCI) wrote…

File is not goimports-ed

Move this to the next group.


dgraph/cmd/live/run.go, line 99 at r1 (raw file):

		"Ignores conflicts on index keys during transaction")
	flag.StringP("auth_token", "a", "",
		"The auth token that is required when a schema file is provided(through the --schema option), " +

100 char limit. And reduce the explanation to a one liner.

Copy link
Author

@gitlw gitlw left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @manishrjain and @golangcibot)


dgraph/cmd/live/run.go, line 25 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Move this to the next group.

Done.


dgraph/cmd/live/run.go, line 99 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

100 char limit. And reduce the explanation to a one liner.

Done.

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @golangcibot)

@manishrjain manishrjain merged commit e7f9158 into hypermodeinc:master Oct 24, 2018
@gitlw gitlw deleted the auth_token_support branch November 8, 2018 22:42
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 19, 2019
Performed testing using the following steps:
1. The alpha server is started with the --auth_token option
2.a The live loader command is run _without_ any change of schema through the --schema option, then the --auth_token option is _not_ required, i.e. the rdf file can still be processed successfully without the --auth_token option
2.b The live loader command is run _with_ a schema change through the --schema option, then the --auth_token option _must_ be provided, otherwise the Alter gRPC call fails with an error complaining about missing the auth token
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants