You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by Angus-Slalom <gi...@git.apache.org> on 2017/06/12 14:54:16 UTC
[GitHub] storm pull request #2157: STORM-2517 storm-hdfs writers can't be subclassed
GitHub user Angus-Slalom opened a pull request:
https://github.com/apache/storm/pull/2157
STORM-2517 storm-hdfs writers can't be subclassed
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Angus-Slalom/storm 1.x-branch-STORM-2517
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2157.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 #2157
----
commit 421336229dbdbe0d2c9a782315c6f2df1e4d0971
Author: Angus Helm <an...@slalom.com>
Date: 2017-05-24T18:27:57Z
STORM-2517 add interface for Writer, make AbstractHDFSWriter properties protected
----
---
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] storm pull request #2157: STORM-2517 storm-hdfs writers can't be subclassed
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2157
---
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] storm issue #2157: STORM-2517 storm-hdfs writers can't be subclassed
Posted by dossett <gi...@git.apache.org>.
Github user dossett commented on the issue:
https://github.com/apache/storm/pull/2157
I hadn't thought about subclassing the writers, instead I thought a new bolt would implements its own writer. @Angus-Slalom what use case do you have in mind? I'm not opposed to the change, just curious.
---
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] storm pull request #2157: STORM-2517 storm-hdfs writers can't be subclassed
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2157#discussion_r125541489
--- Diff: external/storm-hdfs/src/main/java/org/apache/storm/hdfs/bolt/Writer.java ---
@@ -0,0 +1,18 @@
+package org.apache.storm.hdfs.bolt;
--- End diff --
Missing apache license header.
---
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] storm pull request #2157: STORM-2517 storm-hdfs writers can't be subclassed
Posted by harshach <gi...@git.apache.org>.
Github user harshach commented on a diff in the pull request:
https://github.com/apache/storm/pull/2157#discussion_r124087558
--- Diff: external/storm-hdfs/src/main/java/org/apache/storm/hdfs/bolt/AbstractHdfsBolt.java ---
@@ -297,9 +296,21 @@ protected Path getBasePathForNextFile(Tuple tuple) {
abstract protected String getWriterKey(Tuple tuple);
- abstract protected AbstractHDFSWriter makeNewWriter(Path path, Tuple tuple) throws IOException;
+ abstract protected Writer makeNewWriter(Path path, Tuple tuple) throws IOException;
- static class WritersMap extends LinkedHashMap<String, AbstractHDFSWriter> {
+ public interface Writer {
--- End diff --
can you move this into this its own file rather than putting it in HDFSBolt
---
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] storm issue #2157: STORM-2517 storm-hdfs writers can't be subclassed
Posted by dossett <gi...@git.apache.org>.
Github user dossett commented on the issue:
https://github.com/apache/storm/pull/2157
@Angus-Slalom Thanks, that makes sense! Feel free to contribute logging changes as well if you think they are generally applicable :-)
I'm at +1 pending the change Harsha suggested.
---
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] storm issue #2157: STORM-2517 storm-hdfs writers can't be subclassed
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2157
@Angus-Slalom
Could you craft the patch against master? We introduced checkstyle in master branch so forward porting via cherry-pick might not work.
---
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] storm issue #2157: STORM-2517 storm-hdfs writers can't be subclassed
Posted by Angus-Slalom <gi...@git.apache.org>.
Github user Angus-Slalom commented on the issue:
https://github.com/apache/storm/pull/2157
@dossett the AbstractHDFSBolt uses AbstractHDFSWriter throughout, so any implementation of a writer has to inherit from that. For example the makeNewWriter protected function has to return an AbstractHDFSWriter
We've been subclassing the HDFS bolt to add more logging and to make tweaks to our AVRO output
---
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] storm pull request #2157: STORM-2517 storm-hdfs writers can't be subclassed
Posted by Angus-Slalom <gi...@git.apache.org>.
Github user Angus-Slalom commented on a diff in the pull request:
https://github.com/apache/storm/pull/2157#discussion_r125324914
--- Diff: external/storm-hdfs/src/main/java/org/apache/storm/hdfs/bolt/AbstractHdfsBolt.java ---
@@ -297,9 +296,21 @@ protected Path getBasePathForNextFile(Tuple tuple) {
abstract protected String getWriterKey(Tuple tuple);
- abstract protected AbstractHDFSWriter makeNewWriter(Path path, Tuple tuple) throws IOException;
+ abstract protected Writer makeNewWriter(Path path, Tuple tuple) throws IOException;
- static class WritersMap extends LinkedHashMap<String, AbstractHDFSWriter> {
+ public interface Writer {
--- End diff --
updated
---
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] storm issue #2157: STORM-2517 storm-hdfs writers can't be subclassed
Posted by Angus-Slalom <gi...@git.apache.org>.
Github user Angus-Slalom commented on the issue:
https://github.com/apache/storm/pull/2157
Created a pull request on master at https://github.com/apache/storm/pull/2294
---
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.
---