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

Issue 2934: export with uid #3004

Merged
merged 4 commits into from
Feb 12, 2019
Merged

Conversation

codexnull
Copy link
Contributor

@codexnull codexnull commented Feb 11, 2019

Include actual UIDs in export instead of "_:uid" blanks. Fixes #2934.


This change is Reviewable

@codexnull codexnull requested a review from a team February 11, 2019 22:13
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 4 of 4 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @codexnull)


worker/export.go, line 73 at r1 (raw file):

// UIDs like 0x1 look weird but 64-bit ones like 0x0000000000000001 are too long.
var uidFmtStr = "<0x%x>"

I thought based on the discussion in the issue that this was going to be an option and the default was still going to be blank nodes.

Copy link
Contributor Author

@codexnull codexnull 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: all files reviewed, 1 unresolved discussion (waiting on @martinmr)


worker/export.go, line 73 at r1 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…

I thought based on the discussion in the issue that this was going to be an option and the default was still going to be blank nodes.

Yeah, but during the stand-up @manishrjain said to use the UID.

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 4 of 4 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @codexnull and @martinmr)


dgraph/cmd/live/batch.go, line 140 at r1 (raw file):

		time.Sleep(dur)
	case err != y.ErrAborted && err != y.ErrConflict:
		fmt.Printf("Error while mutating. %v\n", s.Message())

switch to colon.


worker/export.go, line 73 at r1 (raw file):

Previously, codexnull (Javier Alvarado) wrote…

Yeah, but during the stand-up @manishrjain said to use the UID.

Yup, UID directly for every export.

Copy link
Contributor Author

@codexnull codexnull 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: 3 of 4 files reviewed, 2 unresolved discussions (waiting on @manishrjain and @martinmr)


dgraph/cmd/live/batch.go, line 140 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

switch to colon.

Done.

@codexnull codexnull merged commit fc9c0c2 into master Feb 12, 2019
@codexnull codexnull deleted the javier/issue2934_export_with_uid branch March 1, 2019 22:53
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 19, 2019
* Export with the actual UIDs instead of blanks.

* Fix awkwardly punctuated error message.

* Fix tests assuming _:uid in export.
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.

3 participants