Dan Burkert has posted comments on this change.

Change subject: WIP: KUDU-1812. Redaction using thread-local boolean approach
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/5557/2/src/kudu/util/flags-test.cc
File src/kudu/util/flags-test.cc:

Line 45:     // command line flags and fails if an unsafe flag is set.
> Nit: or experimental
Done


http://gerrit.cloudera.org:8080/#/c/5557/2/src/kudu/util/logging.cc
File src/kudu/util/logging.cc:

Line 68: TAG_FLAG(log_redact_user_data, experimental);
> Why experimental?
I think it's a wise policy for all new flags to start out as experimental.


Line 82: __thread bool tls_redact_user_data = true;
> Would be nice to add a comment here justifying the default policy.
Done


http://gerrit.cloudera.org:8080/#/c/5557/2/src/kudu/util/logging.h
File src/kudu/util/logging.h:

Line 78: class ScopedDisableRedaction {
> I think it'd be nice to also enforce that there aren't nested calls to KUDU
That's a valid scenario - for instance the web UI calling into a partition 
schema pretty printer.


-- 
To view, visit http://gerrit.cloudera.org:8080/5557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I75392ae36e96c1742e478e82671fd1eac3a0edee
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes

Reply via email to