[
https://issues.apache.org/jira/browse/FLINK-1964?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14537288#comment-14537288
]
ASF GitHub Bot commented on FLINK-1964:
---------------------------------------
Github user carloscurotto commented on the pull request:
https://github.com/apache/flink/pull/666#issuecomment-100671079
Let me also suggest a few other further improvements:
TwitterSource:
- put the default value for queueSize attribute in a constant (we could
call it, DEFAULT_QUEUE_SIZE)
- put the default value for waitSec attribute in a constant (we could call
it, DEFAULT_WAIT_SECONDS)
- i see a couple of methods, that seems to work in paris such as:
- open / close: called at the begining of the function and at the very
end of it
- run / cancel: called on each iteration to perform some work or cancel
the current ongoing work
Is my understanding correct? I am asking since i see that the
TwitterSource is initializing its client connection in the open(...) method but
stops it in the run(...) method. Do those methods works in pairs?
- do you think it is a good idea to assign the client to null after
stopping it in the closeConnection method, so it can be garbage collected as
soon as possible? Also, if closeConnection() throws an exception, isRunning
never changes to false.
- we should check for preconditions in the constructors and public set
methods to avoid receiving null auth propeties or similar cases.
- we could have a private constructor that receives all the possible
parameters and use constructor chaining to avoid code duplication even in the
constructors.
PropertiesUtil:
- I have created a util file to place methods when working with properties
files, i put the load method that loads a properties object form a file path,
we should place this in a more common package, i put it close to the file i
have changed but we should consider moving it to a better place.
General:
- In general i saw a few clases that are missing javadoc and validation of
input parameters.
- Separate the different connectors into different submodules.
> Rework TwitterSource to use a Properties object instead of a file path
> ----------------------------------------------------------------------
>
> Key: FLINK-1964
> URL: https://issues.apache.org/jira/browse/FLINK-1964
> Project: Flink
> Issue Type: Bug
> Components: Streaming
> Affects Versions: 0.9
> Reporter: Robert Metzger
> Assignee: Carlos Curotto
> Priority: Minor
> Labels: starter
>
> The twitter connector is very hard to use on a cluster because it expects the
> property file to be present on all nodes.
> It would be much easier to ask the user to pass a Properties object
> immediately.
> Also, the javadoc of the class stops in the middle of the sentence.
> It was not obvious to me how the two examples TwitterStreaming and
> TwitterTopology differ. Also, there is a third TwitterStream example in the
> streaming examples.
> The documentation of the Twitter source refers to the non existent
> TwitterLocal class.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
