samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Martin Kleppmann" <mkleppm...@linkedin.com>
Subject Re: Review Request 22753: SAMZA-35. Everybody gets a javadoc!
Date Wed, 18 Jun 2014 23:19:19 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22753/#review46136
-----------------------------------------------------------



samza-api/src/main/java/org/apache/samza/checkpoint/Checkpoint.java
<https://reviews.apache.org/r/22753/#comment81343>

    This sentence doesn't make sense.



samza-api/src/main/java/org/apache/samza/config/Config.java
<https://reviews.apache.org/r/22753/#comment81347>

    Not sure I'm aware of this venerability. Is that a reference to Hadoop?



samza-api/src/main/java/org/apache/samza/config/ConfigFactory.java
<https://reviews.apache.org/r/22753/#comment81350>

    Could you explain the purpose of ConfigFactory? Is it to support different file formats
for configuration, or configuration stored in some form that's not a file (e.g. a network
service), or something else?



samza-api/src/main/java/org/apache/samza/job/StreamJob.java
<https://reviews.apache.org/r/22753/#comment81352>

    s/configuration/interface/ ?



samza-api/src/main/java/org/apache/samza/job/StreamJob.java
<https://reviews.apache.org/r/22753/#comment81353>

    Can it just return "this", or is the submitted job different?



samza-api/src/main/java/org/apache/samza/job/StreamJobFactory.java
<https://reviews.apache.org/r/22753/#comment81355>

    Perhaps make explicit that this is where you would plug in a cluster manager/cloud provider
of your choice?



samza-api/src/main/java/org/apache/samza/metrics/Counter.java
<https://reviews.apache.org/r/22753/#comment81356>

    "For example, the number of messages processed since the container was started."



samza-api/src/main/java/org/apache/samza/metrics/Gauge.java
<https://reviews.apache.org/r/22753/#comment81357>

    "For example, the length of a queue or the size of a buffer."



samza-api/src/main/java/org/apache/samza/metrics/MetricsRegistry.java
<https://reviews.apache.org/r/22753/#comment81358>

    What's a group? Any guidance for choosing names?



samza-api/src/main/java/org/apache/samza/metrics/MetricsReporterFactory.java
<https://reviews.apache.org/r/22753/#comment81359>

    Builds a MetricsReporter, not a MetricsReporterFactory.



samza-api/src/main/java/org/apache/samza/metrics/MetricsVisitor.java
<https://reviews.apache.org/r/22753/#comment81360>

    What does it mean to "visit" a metric?



samza-api/src/main/java/org/apache/samza/serializers/SerdeFactory.java
<https://reviews.apache.org/r/22753/#comment81361>

    s/SerdeFactory/Serde/



samza-api/src/main/java/org/apache/samza/serializers/SerdeFactory.java
<https://reviews.apache.org/r/22753/#comment81362>

    typo: otuput



samza-api/src/main/java/org/apache/samza/storage/StorageEngine.java
<https://reviews.apache.org/r/22753/#comment81363>

    Should that be "not deserialized"?



samza-api/src/main/java/org/apache/samza/system/SystemProducer.java
<https://reviews.apache.org/r/22753/#comment81365>

    Typo: "finishe"



samza-api/src/main/java/org/apache/samza/system/SystemProducer.java
<https://reviews.apache.org/r/22753/#comment81366>

    Should that be "sent any remaining messages"?



samza-api/src/main/java/org/apache/samza/system/SystemStream.java
<https://reviews.apache.org/r/22753/#comment81367>

    Perhaps explain that the system name is the name by which it is defined in the job config
(the asterisk in systems.*.samza.factory), and the meaning of the stream name depends on the
system (may be a queue name, or topic name, or file path, etc)



samza-api/src/main/java/org/apache/samza/task/ClosableTask.java
<https://reviews.apache.org/r/22753/#comment81368>

    Typo: "speciy"



samza-api/src/main/java/org/apache/samza/task/ClosableTask.java
<https://reviews.apache.org/r/22753/#comment81369>

    Perhaps note that "close" isn't guaranteed to be called, because container may crash,
power supply may fail, etc. Should be obvious really, but you never know... http://thedailywtf.com/Articles/My-Tales.aspx



samza-api/src/main/java/org/apache/samza/task/WindowableTask.java
<https://reviews.apache.org/r/22753/#comment81373>

    "regardless of whether"? (not sure)
    
    Also, worth pointing out single-threadedness again, no synchronization needed (like your
note on StreamTask)?


- Martin Kleppmann


On June 18, 2014, 10:02 p.m., Jakob Homan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22753/
> -----------------------------------------------------------
> 
> (Updated June 18, 2014, 10:02 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> javadocs
> 
> 
> Diffs
> -----
> 
>   samza-api/src/main/java/org/apache/samza/Partition.java ebb77ed 
>   samza-api/src/main/java/org/apache/samza/checkpoint/Checkpoint.java dcf81bf 
>   samza-api/src/main/java/org/apache/samza/checkpoint/CheckpointManager.java 34f50fd

>   samza-api/src/main/java/org/apache/samza/checkpoint/CheckpointManagerFactory.java 5ce8f35

>   samza-api/src/main/java/org/apache/samza/config/Config.java c42c1c5 
>   samza-api/src/main/java/org/apache/samza/config/ConfigException.java b6ab549 
>   samza-api/src/main/java/org/apache/samza/config/ConfigFactory.java d6d7584 
>   samza-api/src/main/java/org/apache/samza/config/ConfigRewriter.java 7248e8b 
>   samza-api/src/main/java/org/apache/samza/config/MapConfig.java 337e921 
>   samza-api/src/main/java/org/apache/samza/container/SamzaContainerContext.java 5aa7a8f

>   samza-api/src/main/java/org/apache/samza/job/ApplicationStatus.java 49052af 
>   samza-api/src/main/java/org/apache/samza/job/CommandBuilder.java 5ec6433 
>   samza-api/src/main/java/org/apache/samza/job/StreamJob.java f519949 
>   samza-api/src/main/java/org/apache/samza/job/StreamJobFactory.java 4cdcc2c 
>   samza-api/src/main/java/org/apache/samza/metrics/Counter.java 0838df3 
>   samza-api/src/main/java/org/apache/samza/metrics/Gauge.java 3335c15 
>   samza-api/src/main/java/org/apache/samza/metrics/MetricsRegistry.java 9df1ef6 
>   samza-api/src/main/java/org/apache/samza/metrics/MetricsReporter.java d52dfa9 
>   samza-api/src/main/java/org/apache/samza/metrics/MetricsReporterFactory.java 19eb91c

>   samza-api/src/main/java/org/apache/samza/metrics/MetricsType.java e79d4e6 
>   samza-api/src/main/java/org/apache/samza/metrics/MetricsVisitor.java fee0883 
>   samza-api/src/main/java/org/apache/samza/metrics/ReadableMetricsRegistry.java ebea426

>   samza-api/src/main/java/org/apache/samza/serializers/Deserializer.java fe72223 
>   samza-api/src/main/java/org/apache/samza/serializers/Serde.java fab1055 
>   samza-api/src/main/java/org/apache/samza/serializers/SerdeFactory.java a41a922 
>   samza-api/src/main/java/org/apache/samza/serializers/Serializer.java 932e9a5 
>   samza-api/src/main/java/org/apache/samza/storage/StorageEngine.java 96dec9b 
>   samza-api/src/main/java/org/apache/samza/storage/StorageEngineFactory.java da57bf0

>   samza-api/src/main/java/org/apache/samza/system/OutgoingMessageEnvelope.java c8ef980

>   samza-api/src/main/java/org/apache/samza/system/SystemAdmin.java 3976253 
>   samza-api/src/main/java/org/apache/samza/system/SystemConsumer.java a92e301 
>   samza-api/src/main/java/org/apache/samza/system/SystemFactory.java ae33e8e 
>   samza-api/src/main/java/org/apache/samza/system/SystemProducer.java 8967f57 
>   samza-api/src/main/java/org/apache/samza/system/SystemStream.java 0265a2c 
>   samza-api/src/main/java/org/apache/samza/system/SystemStreamPartition.java 5173ebd

>   samza-api/src/main/java/org/apache/samza/system/SystemStreamPartitionIterator.java
62a5eb7 
>   samza-api/src/main/java/org/apache/samza/system/chooser/MessageChooser.java 6d2fa23

>   samza-api/src/main/java/org/apache/samza/system/chooser/MessageChooserFactory.java
6442db9 
>   samza-api/src/main/java/org/apache/samza/task/ClosableTask.java a93cca0 
>   samza-api/src/main/java/org/apache/samza/task/StreamTask.java 00d5efd 
>   samza-api/src/main/java/org/apache/samza/task/TaskContext.java 611507e 
>   samza-api/src/main/java/org/apache/samza/task/TaskCoordinator.java 5049b1b 
>   samza-api/src/main/java/org/apache/samza/task/TaskLifecycleListenerFactory.java 31f32bc

>   samza-api/src/main/java/org/apache/samza/task/WindowableTask.java 1f48eec 
>   samza-api/src/main/java/org/apache/samza/util/BlockingEnvelopeMap.java 7171088 
>   samza-api/src/main/java/org/apache/samza/util/Clock.java e1a77e6 
>   samza-api/src/main/java/org/apache/samza/util/NoOpMetricsRegistry.java 8bc0764 
>   samza-core/src/main/scala/org/apache/samza/system/chooser/BootstrappingChooser.scala
91c1813 
>   samza-kafka/src/main/scala/org/apache/samza/system/kafka/KafkaSystemProducer.scala
5fb7a20 
> 
> Diff: https://reviews.apache.org/r/22753/diff/
> 
> 
> Testing
> -------
> 
> read it.
> 
> 
> Thanks,
> 
> Jakob Homan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message