Skip to content

Conversation

@mohitranka
Copy link
Contributor

@mohitranka mohitranka commented Jul 22, 2016

Fixes #130.


This change is Reviewable

…tations' are created now, if they dont exist already.
@mohitranka mohitranka changed the title Fixes #130. User specified directories for 'postings', 'uids' and 'mu… User specified directories for 'postings', 'uids' and 'mutations' are created now, if they dont exist already. Jul 22, 2016
@coveralls
Copy link

coveralls commented Jul 22, 2016

Coverage Status

Coverage decreased (-0.2%) to 52.419% when pulling 213de86 on feature/create_internal_directories into dd7badb on develop.

@mohitranka
Copy link
Contributor Author

@dgraph-io/team Please wait for my confirmation before reviewing this PR.

…, uids' and 'mutations' directories and only allow user/owner process access to them.
@mohitranka
Copy link
Contributor Author

Please review the PR @dgraph-io/team .

@coveralls
Copy link

coveralls commented Jul 22, 2016

Coverage Status

Coverage decreased (-0.2%) to 52.419% when pulling 5c908d3 on feature/create_internal_directories into dd7badb on develop.

@pawanrawal
Copy link
Contributor

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


cmd/dgraph/main.go, line 328 [r2] (raw file):

  if err != nil {
      log.Fatal("Error while creating the filepath: %v", *postingDir)
      return

Is a return needed here? Also log.Fatal(err) should be enough? Anyways err should be part of the fatal message otherwise user won't know why the command failed.


Comments from Reviewable

@mohitranka
Copy link
Contributor Author

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.


cmd/dgraph/main.go, line 328 [r2] (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Is a return needed here? Also log.Fatal(err) should be enough? Anyways err should be part of the fatal message otherwise user won't know why the command failed.

Done.

Comments from Reviewable

@coveralls
Copy link

coveralls commented Jul 24, 2016

Coverage Status

Coverage decreased (-0.2%) to 52.434% when pulling 81fa0f6 on feature/create_internal_directories into dd7badb on develop.

@pawanrawal
Copy link
Contributor

:lgtm: @manishrjain could you also have a look at this before its merged?


Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@manishrjain
Copy link
Contributor

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


cmd/dgraph/main.go, line 325 [r3] (raw file):

  // Create parent directories for postings, uids and mutations
  var err error
  err = os.MkdirAll(*postingDir, 0700)

What if the directory already exists?


Comments from Reviewable

@mohitranka
Copy link
Contributor Author

Review status: all files reviewed at latest revision, 1 unresolved discussion.


cmd/dgraph/main.go, line 325 [r3] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

What if the directory already exists?

"If path is already a directory, MkdirAll does nothing and returns nil." (https://golang.org/pkg/os/#MkdirAll)

Comments from Reviewable

@manishrjain
Copy link
Contributor

:lgtm: You can merge it now.


Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

mohitranka added a commit that referenced this pull request Jul 27, 2016
@mohitranka
Copy link
Contributor Author

Merged to develop with commit 9d91939

@mohitranka mohitranka closed this Jul 27, 2016
@mohitranka mohitranka deleted the feature/create_internal_directories branch July 27, 2016 06:54
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.

6 participants