Skip to content

fix(bulk): fixing bulk loader when encryption + mtls is enabled#7154

Merged
aman-bansal merged 3 commits intomasterfrom
aman/bulk_loader_fix
Dec 16, 2020
Merged

fix(bulk): fixing bulk loader when encryption + mtls is enabled#7154
aman-bansal merged 3 commits intomasterfrom
aman/bulk_loader_fix

Conversation

@aman-bansal
Copy link
Copy Markdown
Contributor

@aman-bansal aman-bansal commented Dec 16, 2020

This change is Reviewable

@github-actions github-actions Bot added the area/bulk-loader Issues related to bulk loading. label Dec 16, 2020
Copy link
Copy Markdown
Contributor

@danielmai danielmai left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aman-bansal, @manishrjain, and @vvbalaji-dgraph)


dgraph/cmd/bulk/reduce.go, line 134 at r1 (raw file):

		x.WorkerConfig.TLSClientConfig = r.opt.ClientTLSConfig

		if !worker.EnterpriseEnabled() {

Can we do this check in run.go instead of waiting until reduce starts? That way we would've caught this issue earlier.

Copy link
Copy Markdown
Contributor

@danielmai danielmai 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 2 of 3 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aman-bansal, @manishrjain, and @vvbalaji-dgraph)

Copy link
Copy Markdown
Contributor

@danielmai danielmai 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 r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @aman-bansal, @manishrjain, and @vvbalaji-dgraph)

@aman-bansal aman-bansal merged commit 1537085 into master Dec 16, 2020
aman-bansal added a commit that referenced this pull request Dec 16, 2020
* fixing bulk loader when encryption + mtls is enabled

* making enterprise check early

* making tls var localised
danielmai pushed a commit that referenced this pull request Dec 16, 2020
* fixing bulk loader when encryption + mtls is enabled

* making enterprise check early

* making tls var localised
aman-bansal added a commit that referenced this pull request Dec 16, 2020
… (#7155)

* fixing bulk loader when encryption + mtls is enabled

* making enterprise check early

* making tls var localised
@aman-bansal aman-bansal deleted the aman/bulk_loader_fix branch December 17, 2020 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/bulk-loader Issues related to bulk loading.

Development

Successfully merging this pull request may close these issues.

2 participants