-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Allow dgraph migrate with remote mysql server. #4860
Conversation
There was a problem hiding this 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 2 files reviewed, 2 unresolved discussions (waiting on @arijitAD and @manishrjain)
dgraph/cmd/migrate/run.go, line 61 at r1 (raw file):
flag.StringP("separator", "p", ".", "The separator for constructing predicate names") flag.BoolP("quiet", "q", false, "Enable quiet mode to suppress the warning logs") flag.StringP("ip", "", "localhost", "The IP address of the database server.")
You could use h
and p
for shortnames for host and port.
dgraph/cmd/migrate/run.go, line 100 at r1 (raw file):
initDataTypes() pool, err := getPool(user, db, password, ip, port)
minor: the order of args should be ip, port, user, password, db
or user, password, ip, port, db
. It looks odd as of now.
There was a problem hiding this 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 2 files reviewed, 1 unresolved discussion (waiting on @mangalaman93 and @manishrjain)
dgraph/cmd/migrate/run.go, line 61 at r1 (raw file):
Previously, mangalaman93 (Aman Mangal) wrote…
You could use
h
andp
for shortnames for host and port.
p is already used by another flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @arijitAD, @mangalaman93, and @manishrjain)
dgraph/cmd/migrate/run.go, line 61 at r1 (raw file):
Previously, arijitAD (Arijit Das) wrote…
p is already used by another flag.
I'd call it host instead of IP since this supports hostnames as well as IP addresses.
dgraph/cmd/migrate/run.go, line 72 at r2 (raw file):
schemaOutput := conf.GetString("output_schema") dataOutput := conf.GetString("output_data") ip := conf.GetString("ip")
I'd also call this variable host instead of ip.
There was a problem hiding this 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 2 files reviewed, 2 unresolved discussions (waiting on @mangalaman93, @manishrjain, and @martinmr)
dgraph/cmd/migrate/run.go, line 61 at r1 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
I'd call it host instead of IP since this supports hostnames as well as IP addresses.
Done.
dgraph/cmd/migrate/run.go, line 72 at r2 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
I'd also call this variable host instead of ip.
Done.
* Allow dgraph migrate with remote mysql server. * Minor change. * Minor changes. (cherry picked from commit 9bed827)
Fixes #4707
Adds flags to provide the IP address and port of the remote Mysql server.
Example:
dgraph migrate --config config.properties --output_schema schema.txt --output_data sql.rdf --host 10.191.30.251 --port 3306
config.properties content:
user = newuser
password = password
db = testDB
This change is