Re: Review Request 43053: allow warning instead of fail in case of invalid num of partitions in the checkpoint partition

2016-02-18 Thread Yi Pan (Data Infrastructure) 

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


Ship it!




Just one nit.


docs/learn/documentation/versioned/jobs/configuration-table.html (line 238)


It would be nice to put a "CAUTION" note here to indicate that this 
configuration should be used as the last resort if some erronous topic has 
already been created by mistake (i.e. wrong system configuration)


- Yi Pan (Data Infrastructure)


On Feb. 18, 2016, 8:04 p.m., Boris Shkolnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43053/
> ---
> 
> (Updated Feb. 18, 2016, 8:04 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-864
> https://issues.apache.org/jira/browse/SAMZA-864
> 
> 
> Repository: samza
> 
> 
> Description
> ---
> 
> We have a validation code that verifies that checkpoint topic has the right 
> number of partitions (1).
> But, in some environments, it is difficult to repair or delete the invalid 
> topic. 
> This config will allow to by pass this validation (it will issue a warning 
> only) and to continue with a checkpoint topic with an incorrect number of 
> partitions. 
> The checkpoints are written into partion 0.
> 
> 
> Diffs
> -
> 
>   docs/learn/documentation/versioned/jobs/configuration-table.html 
> 67055307329c81c9347f1de352420a48a63634b4 
>   samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala 
> 1a8adae4d30fa198c90e8c177c7f17269c5953cd 
>   
> samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala
>  787de1f62479a098bf251f072fca03bbf92f7c6d 
>   
> samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManagerFactory.scala
>  7db894091284794b7f5fac164eb55b5d78184a36 
>   
> samza-kafka/src/main/scala/org/apache/samza/migration/KafkaCheckpointMigration.scala
>  c6b1fe4bf3c3601502e014d582d86f8ea0850b20 
>   samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala 
> f4311d1cda7c66c66544c5a3ac94a17cae62863a 
>   
> samza-kafka/src/test/scala/org/apache/samza/checkpoint/kafka/TestKafkaCheckpointManager.scala
>  af4051b28df5eeaeaee527a24907a8e66441f743 
> 
> Diff: https://reviews.apache.org/r/43053/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Boris Shkolnik
> 
>

Re: Review Request 43053: allow warning instead of fail in case of invalid num of partitions in the checkpoint partition

2016-02-18 Thread Boris Shkolnik 

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

(Updated Feb. 18, 2016, 8:04 p.m.)


Review request for samza.


Changes
---

updated docs.


Bugs: SAMZA-864
https://issues.apache.org/jira/browse/SAMZA-864


Repository: samza


Description
---

We have a validation code that verifies that checkpoint topic has the right 
number of partitions (1).
But, in some environments, it is difficult to repair or delete the invalid 
topic. 
This config will allow to by pass this validation (it will issue a warning 
only) and to continue with a checkpoint topic with an incorrect number of 
partitions. 
The checkpoints are written into partion 0.


Diffs (updated)
-

  docs/learn/documentation/versioned/jobs/configuration-table.html 
67055307329c81c9347f1de352420a48a63634b4 
  samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala 
1a8adae4d30fa198c90e8c177c7f17269c5953cd 
  
samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala
 787de1f62479a098bf251f072fca03bbf92f7c6d 
  
samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManagerFactory.scala
 7db894091284794b7f5fac164eb55b5d78184a36 
  
samza-kafka/src/main/scala/org/apache/samza/migration/KafkaCheckpointMigration.scala
 c6b1fe4bf3c3601502e014d582d86f8ea0850b20 
  samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala 
f4311d1cda7c66c66544c5a3ac94a17cae62863a 
  
samza-kafka/src/test/scala/org/apache/samza/checkpoint/kafka/TestKafkaCheckpointManager.scala
 af4051b28df5eeaeaee527a24907a8e66441f743 

Diff: https://reviews.apache.org/r/43053/diff/


Testing
---


Thanks,

Boris Shkolnik

Re: Review Request 43053: allow warning instead of fail in case of invalid num of partitions in the checkpoint partition

2016-02-18 Thread Boris Shkolnik 

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

(Updated Feb. 18, 2016, 7:21 p.m.)


Review request for samza.


Changes
---

fixed the test.


Bugs: SAMZA-864
https://issues.apache.org/jira/browse/SAMZA-864


Repository: samza


Description
---

We have a validation code that verifies that checkpoint topic has the right 
number of partitions (1).
But, in some environments, it is difficult to repair or delete the invalid 
topic. 
This config will allow to by pass this validation (it will issue a warning 
only) and to continue with a checkpoint topic with an incorrect number of 
partitions. 
The checkpoints are written into partion 0.


Diffs (updated)
-

  samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala 
1a8adae4d30fa198c90e8c177c7f17269c5953cd 
  
samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala
 787de1f62479a098bf251f072fca03bbf92f7c6d 
  
samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManagerFactory.scala
 7db894091284794b7f5fac164eb55b5d78184a36 
  
samza-kafka/src/main/scala/org/apache/samza/migration/KafkaCheckpointMigration.scala
 c6b1fe4bf3c3601502e014d582d86f8ea0850b20 
  samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala 
f4311d1cda7c66c66544c5a3ac94a17cae62863a 
  
samza-kafka/src/test/scala/org/apache/samza/checkpoint/kafka/TestKafkaCheckpointManager.scala
 af4051b28df5eeaeaee527a24907a8e66441f743 

Diff: https://reviews.apache.org/r/43053/diff/


Testing
---


Thanks,

Boris Shkolnik

Re: Review Request 43053: allow warning instead of fail in case of invalid num of partitions in the checkpoint partition

2016-02-09 Thread Yi Pan (Data Infrastructure) 

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


Ship it!




Ship It!

- Yi Pan (Data Infrastructure)


On Feb. 5, 2016, 9:48 p.m., Boris Shkolnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43053/
> ---
> 
> (Updated Feb. 5, 2016, 9:48 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-864
> https://issues.apache.org/jira/browse/SAMZA-864
> 
> 
> Repository: samza
> 
> 
> Description
> ---
> 
> We have a validation code that verifies that checkpoint topic has the right 
> number of partitions (1).
> But, in some environments, it is difficult to repair or delete the invalid 
> topic. 
> This config will allow to by pass this validation (it will issue a warning 
> only) and to continue with a checkpoint topic with an incorrect number of 
> partitions. 
> The checkpoints are written into partion 0.
> 
> 
> Diffs
> -
> 
>   samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala 
> 1a8adae4d30fa198c90e8c177c7f17269c5953cd 
>   
> samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala
>  787de1f62479a098bf251f072fca03bbf92f7c6d 
>   
> samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManagerFactory.scala
>  7db894091284794b7f5fac164eb55b5d78184a36 
>   
> samza-kafka/src/main/scala/org/apache/samza/migration/KafkaCheckpointMigration.scala
>  c6b1fe4bf3c3601502e014d582d86f8ea0850b20 
>   samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala 
> f4311d1cda7c66c66544c5a3ac94a17cae62863a 
>   
> samza-kafka/src/test/scala/org/apache/samza/checkpoint/kafka/TestKafkaCheckpointManager.scala
>  af4051b28df5eeaeaee527a24907a8e66441f743 
> 
> Diff: https://reviews.apache.org/r/43053/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Boris Shkolnik
> 
>

Re: Review Request 43053: allow warning instead of fail in case of invalid num of partitions in the checkpoint partition

2016-02-09 Thread Yi Pan (Data Infrastructure) 

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




samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala (line 46)


One more nit that I forgot: it would be nice to document why we have to add 
this config: a) Kafka topic auto-creation can not be easily turned off; b) 
Kafka delete topic does not work.


- Yi Pan (Data Infrastructure)


On Feb. 5, 2016, 9:48 p.m., Boris Shkolnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43053/
> ---
> 
> (Updated Feb. 5, 2016, 9:48 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-864
> https://issues.apache.org/jira/browse/SAMZA-864
> 
> 
> Repository: samza
> 
> 
> Description
> ---
> 
> We have a validation code that verifies that checkpoint topic has the right 
> number of partitions (1).
> But, in some environments, it is difficult to repair or delete the invalid 
> topic. 
> This config will allow to by pass this validation (it will issue a warning 
> only) and to continue with a checkpoint topic with an incorrect number of 
> partitions. 
> The checkpoints are written into partion 0.
> 
> 
> Diffs
> -
> 
>   samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala 
> 1a8adae4d30fa198c90e8c177c7f17269c5953cd 
>   
> samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala
>  787de1f62479a098bf251f072fca03bbf92f7c6d 
>   
> samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManagerFactory.scala
>  7db894091284794b7f5fac164eb55b5d78184a36 
>   
> samza-kafka/src/main/scala/org/apache/samza/migration/KafkaCheckpointMigration.scala
>  c6b1fe4bf3c3601502e014d582d86f8ea0850b20 
>   samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala 
> f4311d1cda7c66c66544c5a3ac94a17cae62863a 
>   
> samza-kafka/src/test/scala/org/apache/samza/checkpoint/kafka/TestKafkaCheckpointManager.scala
>  af4051b28df5eeaeaee527a24907a8e66441f743 
> 
> Diff: https://reviews.apache.org/r/43053/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Boris Shkolnik
> 
>

Re: Review Request 43053: allow warning instead of fail in case of invalid num of partitions in the checkpoint partition

2016-02-05 Thread Boris Shkolnik 

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

(Updated Feb. 5, 2016, 9:48 p.m.)


Review request for samza.


Bugs: SAMZA-864
https://issues.apache.org/jira/browse/SAMZA-864


Repository: samza


Description
---

We have a validation code that verifies that checkpoint topic has the right 
number of partitions (1).
But, in some environments, it is difficult to repair or delete the invalid 
topic. 
This config will allow to by pass this validation (it will issue a warning 
only) and to continue with a checkpoint topic with an incorrect number of 
partitions. 
The checkpoints are written into partion 0.


Diffs
-

  samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala 
1a8adae4d30fa198c90e8c177c7f17269c5953cd 
  
samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala
 787de1f62479a098bf251f072fca03bbf92f7c6d 
  
samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManagerFactory.scala
 7db894091284794b7f5fac164eb55b5d78184a36 
  
samza-kafka/src/main/scala/org/apache/samza/migration/KafkaCheckpointMigration.scala
 c6b1fe4bf3c3601502e014d582d86f8ea0850b20 
  samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala 
f4311d1cda7c66c66544c5a3ac94a17cae62863a 
  
samza-kafka/src/test/scala/org/apache/samza/checkpoint/kafka/TestKafkaCheckpointManager.scala
 af4051b28df5eeaeaee527a24907a8e66441f743 

Diff: https://reviews.apache.org/r/43053/diff/


Testing
---


Thanks,

Boris Shkolnik

Re: Review Request 43053: allow warning instead of fail in case of invalid num of partitions in the checkpoint partition

2016-02-05 Thread Boris Shkolnik 


> On Feb. 3, 2016, 2:36 a.m., Jagadish Venkatraman wrote:
> > samza-kafka/src/test/scala/org/apache/samza/checkpoint/kafka/TestKafkaCheckpointManager.scala,
> >  line 84
> > 
> >
> > nit: failOnTopicValidation.

tnx, fixed.


- Boris


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


On Feb. 3, 2016, 1:41 a.m., Boris Shkolnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43053/
> ---
> 
> (Updated Feb. 3, 2016, 1:41 a.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> ---
> 
> We have a validation code that verifies that checkpoint topic has the right 
> number of partitions (1).
> But, in some environments, it is difficult to repair or delete the invalid 
> topic. 
> This config will allow to by pass this validation (it will issue a warning 
> only) and to continue with a checkpoint topic with an incorrect number of 
> partitions. 
> The checkpoints are written into partion 0.
> 
> 
> Diffs
> -
> 
>   samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala 
> 1a8adae4d30fa198c90e8c177c7f17269c5953cd 
>   
> samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala
>  787de1f62479a098bf251f072fca03bbf92f7c6d 
>   
> samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManagerFactory.scala
>  7db894091284794b7f5fac164eb55b5d78184a36 
>   
> samza-kafka/src/main/scala/org/apache/samza/migration/KafkaCheckpointMigration.scala
>  c6b1fe4bf3c3601502e014d582d86f8ea0850b20 
>   samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala 
> f4311d1cda7c66c66544c5a3ac94a17cae62863a 
>   
> samza-kafka/src/test/scala/org/apache/samza/checkpoint/kafka/TestKafkaCheckpointManager.scala
>  af4051b28df5eeaeaee527a24907a8e66441f743 
> 
> Diff: https://reviews.apache.org/r/43053/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Boris Shkolnik
> 
>

Re: Review Request 43053: allow warning instead of fail in case of invalid num of partitions in the checkpoint partition

2016-02-02 Thread Boris Shkolnik 

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

(Updated Feb. 3, 2016, 1:41 a.m.)


Review request for samza.


Changes
---

addressed review comments.


Repository: samza


Description
---

We have a validation code that verifies that checkpoint topic has the right 
number of partitions (1).
But, in some environments, it is difficult to repair or delete the invalid 
topic. 
This config will allow to by pass this validation (it will issue a warning 
only) and to continue with a checkpoint topic with an incorrect number of 
partitions. 
The checkpoints are written into partion 0.


Diffs (updated)
-

  samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala 
1a8adae4d30fa198c90e8c177c7f17269c5953cd 
  
samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala
 787de1f62479a098bf251f072fca03bbf92f7c6d 
  
samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManagerFactory.scala
 7db894091284794b7f5fac164eb55b5d78184a36 
  
samza-kafka/src/main/scala/org/apache/samza/migration/KafkaCheckpointMigration.scala
 c6b1fe4bf3c3601502e014d582d86f8ea0850b20 
  samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala 
f4311d1cda7c66c66544c5a3ac94a17cae62863a 
  
samza-kafka/src/test/scala/org/apache/samza/checkpoint/kafka/TestKafkaCheckpointManager.scala
 af4051b28df5eeaeaee527a24907a8e66441f743 

Diff: https://reviews.apache.org/r/43053/diff/


Testing
---


Thanks,

Boris Shkolnik

Re: Review Request 43053: allow warning instead of fail in case of invalid num of partitions in the checkpoint partition

2016-02-01 Thread Xinyu Liu 

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


Fix it, then Ship it!




LGTM. One minor comment below.


samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala (line 162)


nitpick: the var name of "msg1" seems not very well chosen. Shall we do 
warn(msg + "...") instead?


- Xinyu Liu


On Feb. 1, 2016, 6:25 p.m., Boris Shkolnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43053/
> ---
> 
> (Updated Feb. 1, 2016, 6:25 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> ---
> 
> We have a validation code that verifies that checkpoint topic has the right 
> number of partitions (1).
> But, in some environments, it is difficult to repair or delete the invalid 
> topic. 
> This config will allow to by pass this validation (it will issue a warning 
> only) and to continue with a checkpoint topic with an incorrect number of 
> partitions. 
> The checkpoints are written into partion 0.
> 
> 
> Diffs
> -
> 
>   samza-core/src/main/scala/org/apache/samza/config/JobConfig.scala 
> 1a8adae4d30fa198c90e8c177c7f17269c5953cd 
>   
> samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManager.scala
>  787de1f62479a098bf251f072fca03bbf92f7c6d 
>   
> samza-kafka/src/main/scala/org/apache/samza/checkpoint/kafka/KafkaCheckpointManagerFactory.scala
>  7db894091284794b7f5fac164eb55b5d78184a36 
>   
> samza-kafka/src/main/scala/org/apache/samza/migration/KafkaCheckpointMigration.scala
>  c6b1fe4bf3c3601502e014d582d86f8ea0850b20 
>   samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala 
> f4311d1cda7c66c66544c5a3ac94a17cae62863a 
>   
> samza-kafka/src/test/scala/org/apache/samza/checkpoint/kafka/TestKafkaCheckpointManager.scala
>  af4051b28df5eeaeaee527a24907a8e66441f743 
> 
> Diff: https://reviews.apache.org/r/43053/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Boris Shkolnik
> 
>