Issue Details (XML | Word | Printable)

Key: AVRO-66
Type: New Feature New Feature
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: George Porter
Reporter: George Porter
Votes: 0
Watchers: 2
Operations

If you were logged in you would be able to see more operations.
Avro

add RPC per-call metadata and a plugin API to access it

Created: 30/Jun/09 07:41 PM   Updated: 14/Jul/09 10:50 PM
Return to search
Component/s: c, c++, java, python, spec
Affects Version/s: None
Fix Version/s: 1.0.0

Time Tracking:
Issue & Sub-Tasks
Issue Only
Not Specified

File Attachments:
  Size
Text File Licensed for inclusion in ASF works AVRO-66.1.patch 2009-07-01 05:39 PM George Porter 4 kB
Text File Licensed for inclusion in ASF works AVRO-66.2.patch 2009-07-01 06:02 PM George Porter 4 kB
Text File Licensed for inclusion in ASF works AVRO-66.patch 2009-07-01 08:42 PM Doug Cutting 6 kB
Text File Licensed for inclusion in ASF works AVRO-66.patch 2009-07-01 07:33 PM George Porter 6 kB
Text File Licensed for inclusion in ASF works AVRO-66.patch 2009-07-01 07:27 PM George Porter 6 kB
Text File Licensed for inclusion in ASF works AVRO-66.patch 2009-07-01 07:08 PM George Porter 4 kB
Text File Licensed for inclusion in ASF works AVRO-66.patch 2009-07-01 06:00 PM Doug Cutting 4 kB
Text File Licensed for inclusion in ASF works AVRO-66.patch 2009-06-30 10:36 PM George Porter 2 kB

Resolution Date: 01/Jul/09 09:02 PM

Sub-Tasks  All   Open   
No sub-tasks match this view.

 Description  « Hide
The RPC specification should support per-call metadata maps. Requestor and Responder should have methods that access this per-call metadata.

 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
George Porter added a comment - 30/Jun/09 10:36 PM
This is a simple Java implementation that supports an empty map, but does not include a public API.

Doug Cutting added a comment - 01/Jul/09 04:37 PM
A simpler and more complete way to implement this might be to:
private static final Schema META = Schema.createMap(Schema.create(Schema.Type.BYTES));
private static final GenericDatumReader META_READER = new GenericDatumReader(META);
private static final GenericDatumWriter META_WRITER = new GenericDatumWriter(META);
...
Map<String,ByteBuffer> meta = (Map<String,ByteBuffer>)META_READER.read(null, in);
...
META_WRITER.write(meta, out);

Also, it would be good to have a Python impl committed alongside this, so that interop tests still pass. This could probably do something similar to the above. Do you think you'll be able to do this today?


George Porter added a comment - 01/Jul/09 05:39 PM
This patch incorporates Doug's suggestion to use a GenericDatumReader/Writer to consume and produce a map (vs low-level calls to Encoder and Decoder).

This patch always sends empty maps from the Requestor to the Responder. The Responder simply echos back to the Requestor the same Map it was given. Finally, the Requestor discards the metadata it receives. This will change once we implement the plugin handler mechanism shortly.


Doug Cutting added a comment - 01/Jul/09 06:00 PM
Here's a new version with some minor improvements:
  • responder should not echo meta by default, but return empty;
  • moved 'static final' fields ahead of non-static fields;
  • removed unneeded casts

We still need equivalent Python. I'm not a Python programmer, but will attempt to fake it this afternoon if no one else does first.


George Porter added a comment - 01/Jul/09 06:02 PM
Fixed a small problem with the previous patch by replacing Strings with Utf8s

George Porter added a comment - 01/Jul/09 06:11 PM
Looks like our patches crossed in the mail. I like this patch, but perhaps we should switch "String" to "Utf8"?

Doug Cutting added a comment - 01/Jul/09 06:25 PM
Yes, Utf8 is correct. Will you repair or should I?

George Porter added a comment - 01/Jul/09 06:35 PM
I will
-George

George Porter added a comment - 01/Jul/09 07:08 PM
Updated Doug's patch to change Strings to Utf8s

George Porter added a comment - 01/Jul/09 07:27 PM
This patch includes the Java changes as before, but also changes to Python.

All tests pass, and the interop tests pass as well.

I'm not a Python programmer, and so I'm not sure if there is a better way to manifest a Map schema in the program than what I did, which was to read it out of an on-disk file called Meta.avsc


George Porter added a comment - 01/Jul/09 07:33 PM
Forgot to include Meta.avsc in the previous patch.

Doug Cutting added a comment - 01/Jul/09 08:42 PM
Here's a version that includes Python support.

Doug Cutting added a comment - 01/Jul/09 08:45 PM
I didn't see your patch when I submitted mine. You missed the error case in the Python responder. I inlined the schema, rather than reading it from a file. It seems simple enough that it's not worth reading from a file. What do you think?

George Porter added a comment - 01/Jul/09 08:48 PM
It looks like our patches crossed in the mail again...

I'm happy with using the Python code that you just committed vs mine, since mine included a separate Meta.avsc file that isn't necessary.


Doug Cutting added a comment - 01/Jul/09 09:02 PM
I just committed this. Thanks, George.

Sharad Agarwal added a comment - 02/Jul/09 04:43 AM
Thanks George and Doug for quickly getting in python support.