[GitHub] drill pull request #1190: DRILL-5937: ExecConstants: changed comment, timeou...
Github user asfgit closed the pull request at: https://github.com/apache/drill/pull/1190 ---
[GitHub] drill pull request #1190: DRILL-5937: ExecConstants: changed comment, timeou...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1190#discussion_r177385544
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---
@@ -557,8 +557,7 @@ private ExecConstants() {
public static final LongValidator CODE_GEN_EXP_IN_METHOD_SIZE_VALIDATOR
= new LongValidator(CODE_GEN_EXP_IN_METHOD_SIZE);
/**
- * Timeout for create prepare statement request. If the request exceeds
this timeout, then request is timed out.
- * Default value is 10mins.
+ * Default value is 30 seconds.
--- End diff --
Maybe we should remove comment about the default as well? Actual default
value can be found in drill-module.conf and if it changes, we won't need to
modify it here.
---
[GitHub] drill pull request #1190: DRILL-5937: ExecConstants: changed comment, timeou...
Github user vrozov commented on a diff in the pull request:
https://github.com/apache/drill/pull/1190#discussion_r177300882
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---
@@ -558,7 +558,7 @@ private ExecConstants() {
/**
* Timeout for create prepare statement request. If the request exceeds
this timeout, then request is timed out.
- * Default value is 10mins.
+ * Default value is 30 seconds.
--- End diff --
I refer to the entire comment. "If the request exceeds this timeout, then
request is timed out" does not add any value, it is simply a definition for a
timeout. The same for `CREATE_PREPARE_STATEMENT_TIMEOUT_MILLIS` and "Timeout
for create prepare statement request".
---
[GitHub] drill pull request #1190: DRILL-5937: ExecConstants: changed comment, timeou...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1190#discussion_r177152303
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---
@@ -558,7 +558,7 @@ private ExecConstants() {
/**
* Timeout for create prepare statement request. If the request exceeds
this timeout, then request is timed out.
- * Default value is 10mins.
+ * Default value is 30 seconds.
--- End diff --
Agree, with Vlad, it better to remove this sentence at all: `Default value
is 30 seconds.`
---
[GitHub] drill pull request #1190: DRILL-5937: ExecConstants: changed comment, timeou...
Github user vrozov commented on a diff in the pull request:
https://github.com/apache/drill/pull/1190#discussion_r177103076
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---
@@ -558,7 +558,7 @@ private ExecConstants() {
/**
* Timeout for create prepare statement request. If the request exceeds
this timeout, then request is timed out.
- * Default value is 10mins.
+ * Default value is 30 seconds.
--- End diff --
Is it 30 seconds or 10 seconds? In any case, I don't think that the entire
comment provides any additional info that cannot be inferred from the code
itself.
---
