> On 2010-06-09 17:54:20, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 111
> > <http://review.hbase.org/r/128/diff/2/?file=1157#file1157line111>
> >
> >     do we need to make these fields nullable?  usually they are true/false 
> > in the java code.  
> >     
> >     Is this some semi-mechanical translation from a java api?

I use the same Avro record for table creation and modification as well as 
description. For create table, I want the fields to be nullable because the 
user should not have to specify a value.


> On 2010-06-09 17:54:20, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 94
> > <http://review.hbase.org/r/128/diff/2/?file=1157#file1157line94>
> >
> >     the compression can never be null, because the "NONE" is the catch all 
> > here.

Same as below: I use the same record for family creation, modification, and 
description. Avro currently doesn't have default values on write, so making 
this field nullable means we can do smart things if the user doesn't specify a 
compression algorithm during Family creation.


> On 2010-06-09 17:54:20, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 78
> > <http://review.hbase.org/r/128/diff/2/?file=1157#file1157line78>
> >
> >     same as deadServerNames.

Yeah I should make these 0-length arrays.


> On 2010-06-09 17:54:20, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 73
> > <http://review.hbase.org/r/128/diff/2/?file=1157#file1157line73>
> >
> >     couldnt you use a empty string if there are no dead server names?  im 
> > not sure if arrays can be 0 length in avro :-)

Will make a 0-length array


> On 2010-06-09 17:54:20, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 66
> > <http://review.hbase.org/r/128/diff/2/?file=1157#file1157line66>
> >
> >     technically the serverName is the serverAddress + startCode... in the 
> > Java code is isnt fully exposed.  Not sure what we want to do here, but 
> > this is probably fine as is.

Yeah since Avro records don't have methods, you can think of this field as a 
materialization of the Java logic.


> On 2010-06-09 17:54:20, Ryan Rawson wrote:
> > trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro, line 34
> > <http://review.hbase.org/r/128/diff/2/?file=1157#file1157line34>
> >
> >     you can probably just use 'hostname' and 'port'.  There was a recent 
> > patch in trunk that is attempting to get rid of IP addresses (they cause 
> > issues when they dont align with DNS names, etc) and generally move us to a 
> > DNS name world.

Let me know what you want me to do here. I was just copying the fields directly 
from the Java objects.


- Jeff


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.hbase.org/r/128/#review164
-----------------------------------------------------------


On 2010-06-08 20:05:01, Jeff Hammerbacher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://review.hbase.org/r/128/
> -----------------------------------------------------------
> 
> (Updated 2010-06-08 20:05:01)
> 
> 
> Review request for hbase.
> 
> 
> Summary
> -------
> 
> Initial patch; some javadoc and tests missing, but I wanted to get some 
> initial feedback on the approach. My apologies for sticking a patch on the 
> JIRA before the review. I should have read further on the HowToContribute 
> JIRA.
> 
> 
> This addresses bug HBASE-2400.
> 
> 
> Diffs
> -----
> 
>   trunk/bin/hbase 952782 
>   trunk/pom.xml 952782 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroServer.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/AvroUtil.java PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AAlreadyExists.java
>  PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AClusterStatus.java
>  PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AColumn.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AColumnFamilyDescriptor.java
>  PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AColumnValue.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ACompressionAlgorithm.java
>  PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ADelete.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AFamilyDescriptor.java
>  PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AGet.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AIOError.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AIllegalArgument.java
>  PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AMasterNotRunning.java
>  PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/APut.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ARegionLoad.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AResult.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AResultEntry.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AScan.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AServerAddress.java
>  PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AServerInfo.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/AServerLoad.java 
> PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ATableDescriptor.java
>  PRE-CREATION 
>   
> trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ATableExists.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/ATimeRange.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/HBase.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/IOError.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/generated/TCell.java 
> PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.avpr PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/hbase.genavro PRE-CREATION 
>   trunk/src/main/java/org/apache/hadoop/hbase/avro/package.html PRE-CREATION 
>   trunk/src/test/java/org/apache/hadoop/hbase/avro/TestAvroServer.java 
> PRE-CREATION 
> 
> Diff: http://review.hbase.org/r/128/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jeff
> 
>

Reply via email to