You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by joshelser <gi...@git.apache.org> on 2016/10/06 18:02:51 UTC

[GitHub] accumulo pull request #161: ACCUMULO-4453 Remove constructor code duplicatio...

GitHub user joshelser opened a pull request:

    https://github.com/apache/accumulo/pull/161

    ACCUMULO-4453 Remove constructor code duplication and spammy DEBUG lo\u2026

    \u2026g message

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/joshelser/accumulo ACCUMULO-4453-scan-data-source

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/accumulo/pull/161.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 #161
    
----
commit d7ea2268f8348580562709bf44dbdcf1ec9061d8
Author: Josh Elser <el...@apache.org>
Date:   2016-10-06T18:00:01Z

    ACCUMULO-4453 Remove constructor code duplication and spammy DEBUG log message

----


---
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] accumulo issue #161: ACCUMULO-4453 Remove constructor code duplication and s...

Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/161
  
    Thanks, @phrocker! I mused around with making the change you suggested, but, in the end, I just felt like making another many-arg constructor was not helping simplify anything. Can revisit later if desired.


---
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] accumulo issue #161: ACCUMULO-4453 Remove constructor code duplication and s...

Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/161
  
    Oh, `mvn verify -Psunny` passed with this change for me locally.


---
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] accumulo pull request #161: ACCUMULO-4453 Remove constructor code duplicatio...

Posted by phrocker <gi...@git.apache.org>.
Github user phrocker commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/161#discussion_r82305029
  
    --- Diff: server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java ---
    @@ -74,32 +74,26 @@
     
       ScanDataSource(Tablet tablet, Authorizations authorizations, byte[] defaultLabels, HashSet<Column> columnSet, List<IterInfo> ssiList,
           Map<String,Map<String,String>> ssio, AtomicBoolean interruptFlag, SamplerConfiguration samplerConfig, long batchTimeOut, String context) {
    -    this.tablet = tablet;
    -    expectedDeletionCount = tablet.getDataSourceDeletions();
    -    this.options = new ScanOptions(-1, authorizations, defaultLabels, columnSet, ssiList, ssio, interruptFlag, false, samplerConfig, batchTimeOut, context);
    -    this.interruptFlag = interruptFlag;
    -    this.loadIters = true;
    -    log.debug("new scan data source, tablet: {}, options: {}, interruptFlag: {}, loadIterators: {}", this.tablet, this.options, this.interruptFlag,
    -        this.loadIters);
    +    this(tablet, tablet.getDataSourceDeletions(), new ScanOptions(-1, authorizations, defaultLabels, columnSet, ssiList, ssio, interruptFlag, false,
    +        samplerConfig, batchTimeOut, context), interruptFlag, true);
       }
     
       ScanDataSource(Tablet tablet, ScanOptions options) {
    -    this.tablet = tablet;
    -    expectedDeletionCount = tablet.getDataSourceDeletions();
    -    this.options = options;
    -    this.interruptFlag = options.getInterruptFlag();
    -    this.loadIters = true;
    -    log.debug("new scan data source, tablet: {}, options: {}, interruptFlag: {}, loadIterators: {}", this.tablet, this.options, this.interruptFlag,
    -        this.loadIters);
    +    this(tablet, tablet.getDataSourceDeletions(), options, options.getInterruptFlag(), true);
       }
     
       ScanDataSource(Tablet tablet, Authorizations authorizations, byte[] defaultLabels, AtomicBoolean iFlag) {
    +    this(tablet, tablet.getDataSourceDeletions(), new ScanOptions(-1, authorizations, defaultLabels, EMPTY_COLS, null, null, iFlag, false, null, -1, null),
    --- End diff --
    
    it's unrelated but would it make sense to have a constructor for scan options that accepts only the configurable arguments in this ScanDataSource constructor? Other than this line it seems fine. 


---
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] accumulo issue #161: ACCUMULO-4453 Remove constructor code duplication and s...

Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/161
  
    @dlmarion do you have a moment to take a glance?


---
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] accumulo pull request #161: ACCUMULO-4453 Remove constructor code duplicatio...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/accumulo/pull/161


---
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.
---