Skip to content

Issue 2934: export with uid#3004

Merged
codexnull merged 4 commits intomasterfrom
javier/issue2934_export_with_uid
Feb 12, 2019
Merged

Issue 2934: export with uid#3004
codexnull merged 4 commits intomasterfrom
javier/issue2934_export_with_uid

Conversation

@codexnull
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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