You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by sohami <gi...@git.apache.org> on 2017/04/05 18:09:55 UTC
[GitHub] drill pull request #807: DRILL-5415: Improve Fixture Builder to configure cl...
GitHub user sohami opened a pull request:
https://github.com/apache/drill/pull/807
DRILL-5415: Improve Fixture Builder to configure client properties an\u2026
\u2026d keep collection type properties for server
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sohami/drill DRILL-5415
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/807.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #807
----
commit 1608487142f6d1f22e70eed00b508b8006200325
Author: Sorabh Hamirwasia <sh...@maprtech.com>
Date: 2017-04-05T18:04:58Z
DRILL-5415: Improve Fixture Builder to configure client properties and keep collection type properties for server
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #807: DRILL-5415: Improve Fixture Builder to configure cl...
Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:
https://github.com/apache/drill/pull/807#discussion_r110512302
--- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
@@ -203,7 +219,15 @@ private void createConfig(FixtureBuilder builder) throws Exception {
if (builder.configResource != null) {
config = DrillConfig.create(builder.configResource);
} else if (builder.configProps != null) {
- config = DrillConfig.create(configProperties(builder.configProps));
+ // Create Config object out of all the String type configuration.
--- End diff --
Fixed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #807: DRILL-5415: Improve Fixture Builder to configure cl...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/807#discussion_r110238313
--- Diff: exec/java-exec/src/test/java/org/apache/drill/test/ClusterFixture.java ---
@@ -203,7 +219,15 @@ private void createConfig(FixtureBuilder builder) throws Exception {
if (builder.configResource != null) {
config = DrillConfig.create(builder.configResource);
} else if (builder.configProps != null) {
- config = DrillConfig.create(configProperties(builder.configProps));
+ // Create Config object out of all the String type configuration.
--- End diff --
Suggestion to keep things tidy. Keep all properties (simple and complex) in the builder's `configProps` object. Then, in `configProperties`, do the mapping as required to create the `DrillConfig`. If a value is a collection, use the new code to set the value. Else, convert the value using `toString()`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill issue #807: DRILL-5415: Improve Fixture Builder to configure client pr...
Posted by amansinha100 <gi...@git.apache.org>.
Github user amansinha100 commented on the issue:
https://github.com/apache/drill/pull/807
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #807: DRILL-5415: Improve Fixture Builder to configure cl...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/807#discussion_r110240217
--- Diff: exec/java-exec/src/test/java/org/apache/drill/test/FixtureBuilder.java ---
@@ -117,7 +122,27 @@ public FixtureBuilder configProperty(String key, Object value) {
if (configProps == null) {
configProps = defaultProps();
}
- configProps.put(key, value.toString());
+
+ if(value instanceof Collection<?>) {
--- End diff --
Can we just leave the collection in the single `configProps` object, and do the mapping when creating the final config? That is, for the DrillConfig, make the collection test when building DrillConfig. See comment above.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #807: DRILL-5415: Improve Fixture Builder to configure cl...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/807
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #807: DRILL-5415: Improve Fixture Builder to configure cl...
Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on a diff in the pull request:
https://github.com/apache/drill/pull/807#discussion_r110512304
--- Diff: exec/java-exec/src/test/java/org/apache/drill/test/FixtureBuilder.java ---
@@ -117,7 +122,27 @@ public FixtureBuilder configProperty(String key, Object value) {
if (configProps == null) {
configProps = defaultProps();
}
- configProps.put(key, value.toString());
+
+ if(value instanceof Collection<?>) {
--- End diff --
Fixed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---