My 2c are that I think we could potentially ship without refactoring and just keeping them as non-configurable settings for other languages with the explicit plan to refactor post-release (Build). Ideally we could do both, but also aware of the fact we are up against a time crunch. That said there's always the risk we end up breaking people in the preview by waiting longer. I'll let @fabiocav @anirudhgarg and @pragnagopa weigh in on urgency of the rich types in attribute.

from azure-functions-kafka-extension.

If you have issues or questions on how to setup bindings to support open generics, we can provide some guidance on that as well.

from azure-functions-kafka-extension.

@fabiocav can you setup some time with @fbeltrao for early next week? I believe @anirudhgarg was going to set that up but let's get it started so we understand what we're going with.

And yes - I'm currently using a snapshot on MyGet so demos are safe. I think we definitely need to ship something at build. Given the noise around other announcements that aren't in this repo, Kafka is a key piece. Let's see how far we can get.

from azure-functions-kafka-extension.

Good news is I just finished testing this in JavaScript / Typescript. So we should have C#, JavaScript / TypeScript today. Would be great if we could enable Python and Java by Build

from azure-functions-kafka-extension.

https://github.com/jeffhollan/functions-ts-kafka-twitter

from azure-functions-kafka-extension.

@jeffhollan : So looks like Javascript it works seamlessly with the native C# bindings which is great.
So I chatted with @maiqbal11 today and yes Python needs a definition of trigger type and I am attempting to get a basic datatype of string working before I attempt rich types.

from azure-functions-kafka-extension.

KafkaEventData is a strong type that is/will be defined in azure-functions-kafka-extension sdk. We do not have support for strong types in non dotnet languages. Right now, out-of-proc languages cannot bind to type KafkaEventData . Eventually when do not support strong types, each language needs to add this type.

from azure-functions-kafka-extension.

Agree, it makes cleaner. But, the part I am not understanding is why would it make easier for other (out-of-proc) languages?

from azure-functions-kafka-extension.

Indeed, the key thing here is about the pattern and exposing language specific properties in the function metadata, in this case, the other benefit of revisiting the design is a better experience for .NET developers as well, as you can acually honor the types they want to use and expose the Key and Value properties using them, with a generic version of the type.

from azure-functions-kafka-extension.

Yes. Will close it.

from azure-functions-kafka-extension.

Thanks @TsuyoshiUshio for looking into this. Looking at the KafkaTriggerAttribute it looks like only some optional attributes are strongly typed. Do you know if this blocks us from doing any Java support or just we'd need to refactor before we exposed all config options?

from azure-functions-kafka-extension.

We had a meeting with @fabiocav and @pragnagopa for support Java implementation today(I'm in charge java) . I ask them to how to implement it in this case. Their answer was we need to refactor the API little bit. (it is only works for C#) So, I'm trying to have a meeting with them and @fbeltrao to clarify, what is required for that. I'd like to make it minimum impact. (or, Language support is P1, so if we need to publish this soon, then do it as alpha or beta, then we can introduce breaking change later.
It is simply blocks multi language support(Java, python, javascript). We might discuss the decision with @ryancrawcour . At any way, I'll make it clear that how much effort is required for the refactoring. Then we can make decision.

from azure-functions-kafka-extension.

How can you make it for JavaScript/TypeScript for this? How they implement (Type) part? If they are OK, Java might be Ok. Can you share the code?

from azure-functions-kafka-extension.

Interested in Javascript implementation for this as well. Also the contribution strategy varies across languages. Python has a different way to glue in binding parts compared to Java for example.

from azure-functions-kafka-extension.

And yes @priyaananthasankar I was hoping a def main(event: str) -> None would work with Python as well with the latest changes for the recent commit of allowing generic bindings but unfortunately it throws an exception about how KafkaTrigger isn't a recognized binding

from azure-functions-kafka-extension.

@jeffhollan where can I find the mapping definition between java script and C#? Don't need it for javascript? For example, Java is something like this. (On going. not finished just for share) 03e5985 Static typed language and dynamic typed might be difference. (I only know how to mapping between Java and C#)

from azure-functions-kafka-extension.

Yes each language has it's own quirks:

• .NET: Rich types supported for trigger. Attributes and data type bindings that can convert as needed. The attributes generate the function.json
• JavaScript: Pretty much just interfaces with all the .NET stuff out of the box but to the binding it "looks" like something that is just asking for string types for everything (or byte[] and soon Stream). function.json is hand crafted
• Java: Almost the same as JavaScript. The inputs and outputs are always just String or byte[]. However, Java has attributes so we have to define a Java attribute that can map to a function.json that can be loaded by the C# extension. That's the work that this issue is about I believe. Making it so the attribute config can be serialized and deserialized from Java into the function.json
• Python: This one is pretty unique. It has "rich" bindings, meaning instead of just passing back and for str or byte[] it actually maps to Python implementations of each data type. This also means for new bindings you have to add some Python class mapping stuff that @priyaananthasankar was mentioning.

I believe that's the accurate mapping. So Java work should be small, Python work is likely most complex (but provides richest experience)

from azure-functions-kafka-extension.

Thank you for clarify it. However, it could be a problem for other languages. (Not only for Java)Look at the Readme of this project. This Kafka trigger support several protocols.

Avro bindings

public static void User(
    [KafkaTrigger("BrokerList", "users", ValueType=typeof(UserRecord), ConsumerGroup = "myGroupId")] KafkaEventData[] kafkaEvents,
    ILogger logger)
  : 

Protocol Buffer binidngs

    [FunctionName(nameof(ProtobufUser))]
    public static void ProtobufUser(
        [KafkaTrigger("BrokerList", "protoUser", ValueType=typeof(ProtoUser), ConsumerGroup = "myGroupId")] KafkaEventData[] kafkaEvents,
        ILogger logger)
    {

The point

They both have ValueType as type. It is the type of C#. If we serialize it, it will be something like this. `"TypeSerializeTesting.SomeClass, TypeSerializeTesting, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null". Which means, this notation expect that C# side custom class definition. The implementation switches the listener for each protocol by this Type (if specific interface is implemented or not) It is almost impossible to pass the value which is a custom class on the C# side from language extension side. However, the final API of the kafka is just require string. If we can refactor this mechanism to the other way, then we can support all languages include these protocol support scenario.

When I chat with @ryancrawcour on the Teams channel, he said @fbeltrao is on vacation. So I can do refactor it.

from azure-functions-kafka-extension.

@TsuyoshiUshio I don't think you can reuse the deserialisers for other languages so easily. In my understanding you should work with byte[] or string. Deserialisation is then implemented at the function level.

from azure-functions-kafka-extension.

@fbeltrao I’ll ask them if we can skip the definition for the java side. You Switch the logic by the interface type. Is it only for deserializing in C#? Then I should ignore the KeyType and ValueType, and go without it(if possible) I’ll ask them tomorrow.

from azure-functions-kafka-extension.

Yes, the type is for deserialization. Skipping gets you byte[] as body and the key is ignored

from azure-functions-kafka-extension.

There is already a dataType property which can be used specify input type. Updating KafkaEventData to use generic types for key and value will keep the api cleaner.
C# functions can bind to KafkaEventData. Out-of-proc languages will use string/byte[]

from azure-functions-kafka-extension.

As in KafkaEventData<K,V> ?
How would other languages use string or byte[]? Just a converter from KafkaEventData<K,V> to byte[] and string will be enough?

from azure-functions-kafka-extension.

Thanks for the info. So, in the current state, we need to be able to bind to byte[] and string to support all out-of-proc languages?

Is the current implementation usage of converters addressing this issue or do we need changes?

from azure-functions-kafka-extension.

Following example from SingleItemTrigger.cs

 [KafkaTrigger("LocalBroker", Constants.StringTopicWithOnePartitionName, ConsumerGroup = nameof(SingleItemTrigger), ValueType = typeof(string))] KafkaEventData kafkaEvent,
            ILogger log)

is only valid for C#. ValueType property on KafkaTrigger is redundant ad type can be inferrred from the KafkaEventData <K,V>
Removing value type will clean up KafkaTrigger api.

from azure-functions-kafka-extension.

Two issues with the current implementation:

• Introduces a new property valueType for out-of-proc languages which is unnecessary when the same functionality can be achieved with dataType property
• Establishes a pattern for exposing Type when not needed. This would force a Java implementation of strong type KafkaEventData also to use object key and object value instead of using generics to infer the type requested by the function code.

from azure-functions-kafka-extension.

@fabiocav guidance on binding with generics would be great

from azure-functions-kafka-extension.

I also want to know that.

from azure-functions-kafka-extension.

Hi @fabiocav and @pragnagopa ,
Should we change the Attribute before publish it for multi language support? I'd like to know if it is mandatory or recommendation. Can we implement java annotation with ignoring the KeyType and ValueType for current implementation ? Or should we start with refactoring?

from azure-functions-kafka-extension.

Sure. It must be the safe way for build. We are creating a branch for the refactor (it is huge change) and try not to merge master until the build finished. For the Rich types, we don't need to support "Type" type. :) It is .NET specific and it is impossible to handle from other languages. I don't want let them busy for it. :) Anyway, thank you for the advice. I'll follow your advice, release the current version then refactor/release after the build. Other question is, can I implement java version without implementing KeyType and ValueType? It has default, and javascript version works to ignore these. It might work. If it doesn't work. I'll contribute the refactoring first.

from azure-functions-kafka-extension.

Agree with @jeffhollan. Please create a new branch, do the refactor and submit a PR with all tests passing, and any new tests required by this change. Ideally at least one additional language that is requiring this change.

from azure-functions-kafka-extension.

@ryancrawcour it will benefit all languages. Today the Kafka trigger exposes some config that is only accessible in C#.

Spoke to @pragnagopa - understandably there's some hesitancy of shipping a preview with a feature we know we're going to have to break / refactor in the next few weeks. Was wondering about the following approach:

1. Create a new branch where we refactor the code to expose this config in a type-agnostic way
2. Remove the attributes properties that are rich types in master. That way no one starts using them short term while we refactor on this other branch. This is what we ship in the next week or two.

Means we'd likely ship without the Type mapping stuff and the Avro stuff. Also means the MVP would only have Plain authentication mode and plain text protocol. FWIW the default settings seem to work great with these Kubernetes clusters I've been building demos and docs are, so I think this is ok and we know we'll light these features back up once we refactor.

Thoughts? Does this work?

from azure-functions-kafka-extension.

Sure. It'll work. Just wondering if we bother shipping anything until this refactor is complete.
It feels like many Kafka customers use Avro and authentication, so removing this will have a large impact.

from azure-functions-kafka-extension.

It still works with Avro even without this right? It would just require in the interim you do the Avro decoding within the execution instead of relying on the trigger to automatically do it for you. We've got Event Hubs customers who do this today so would be at parity. But I'm open to thoughts on it.

from azure-functions-kafka-extension.

The change will breaks these things.

• e2e testing
• samples
• documentation
• some logic change required

Currently, KafkaTriggerAttribute.KeyType has 4 references, and ValueType has 17 references. Also we need to change the KafkaAttribute as well which has 4 references for KeyType and 12 references for ValueType. I recommend to ask @fbeltrao is back to this conversation.

from azure-functions-kafka-extension.

I dunno. You said it would mean removing Avro :)
The change has to be done, donkeys get it done.
The sooner we get it done the sooner we get this shipped and customers using it.

Now the question is who is going to take this on?

from azure-functions-kafka-extension.

I am looking at the required changes at the moment. I am stuck with binding with generics. If the team could provide a guidance I would be able to estimate how long it would take to do the refactor in a second branch.

Nevertheless, I would recommend making an internal branch/release with the current version to make sure you can always go back to the working version you are testing/demoing on top of.

from azure-functions-kafka-extension.

@fbeltrao already submit a PR for this. Amazing. I see the Type was removed from Attributes.
@fabiocav and @pragnagopa , Could you review it, please?

from azure-functions-kafka-extension.

@fbeltrao with the merging of #82 can we now consider this issue resolved?

from azure-functions-kafka-extension.