search

arcus-azure / arcus.eventgrid

Azure Event Grid development in a breeze
https://eventgrid.arcus-azure.net/
MIT License
17 stars 6 forks source link

FEAT: using abstracted event for all EventGrid event parsing #112

Closed stijnmoreels closed 4 years ago

stijnmoreels commented 4 years ago

Using our abstracted model Event to represent both CloudEvents and EventGrid events, we can leverage this further by simplifying the parsing to a more "I don't care" approach.

Closes #106

arcus-automation commented 4 years ago

A new preview package for Arcus.EventGrid.All is available on MyGet.

You can pull it locally via the CLI:

PM> Install-Package Arcus.EventGrid.All -Version 20200124.0.0-PR-112 -Source https://www.myget.org/F/arcus/api/v3/index.json
arcus-automation commented 4 years ago

A new preview package for Arcus.EventGrid.All is available on MyGet.

You can pull it locally via the CLI:

PM> Install-Package Arcus.EventGrid.All -Version 20200127.0.0-PR-112 -Source https://www.myget.org/F/arcus/api/v3/index.json
tomkerkhove commented 4 years ago

I think we should take a step back and think how we'd like to approach this.

We need to achieve following things:

What I'm trying to say is that now both deserialization & representation are tightly couple, but maybe we need to get rid of that.

Just thinking out loud.

stijnmoreels commented 4 years ago

What I'm trying to say is that now both deserialization & representation are tightly couple, but maybe we need to get rid of that.

Yes, so you don't have to worry about which type it is you deserialize? I think we had that .ToEvent and currently on master .Parse that would do that for us. Or am I misinterpreting you :smile:

arcus-automation commented 4 years ago

A new preview package for Arcus.EventGrid.All is available on MyGet.

You can pull it locally via the CLI:

PM> Install-Package Arcus.EventGrid.All -Version 20200210.0.0-PR-112 -Source https://www.myget.org/F/arcus/api/v3/index.json
tomkerkhove commented 4 years ago

I think it's not really clear what we are trying to achieve here:

  1. EventParser.Parse(rawEvent) parses events into EventGridEventBatch<Event>
    • Under the hood it uses multiple parses specific for contract type & version
  2. Event is the universal representation of an event, regardless of it's initial schema
    • It should support .AsEventGridEvent() & AsCloudEvent() based on the generic information

And on a side note, since we are working towards 3.0; maybe we should also rename EventGridEventBatch<Event> to EventBatch<Event> or is that one bridge too far? (not in this PR ofcourse)

stijnmoreels commented 4 years ago

I think it's not really clear what we are trying to achieve here:

  1. EventParser.Parse(rawEvent) parses events into EventGridEventBatch<Event>

    • Under the hood it uses multiple parses specific for contract type & version

  2. Event is the universal representation of an event, regardless of it's initial schema

    • It should support .AsEventGridEvent() & AsCloudEvent() based on the generic information

That's gonna be not entirely correct because the CloudEvents and EventGrid events are not isomorphic. Meaning, there's loss of information during conversion. How do we plan to accomplish that? For example, the .Source of the CloudEvents doesn't exists in the EventGrid events?

I just published an update where I concatenate the EventGrid topic with some versions, but that's not enitrely correct.

And on a side note, since we are working towards 3.0; maybe we should also rename EventGridEventBatch<Event> to EventBatch<Event> or is that one bridge too far? (not in this PR ofcourse)

Yes, other PR, and I agree.

arcus-automation commented 4 years ago

A new preview package for Arcus.EventGrid.All is available on MyGet.

You can pull it locally via the CLI:

PM> Install-Package Arcus.EventGrid.All -Version 20200211.0.0-PR-112 -Source https://www.myget.org/F/arcus/api/v3/index.json
stijnmoreels commented 4 years ago

Added issue for renaming event batch: #113.

stijnmoreels commented 4 years ago

@tomkerkhove my maintainable message pump testing approach is locally almost done, but still has the issue of JSON parsing, which is fixed with this PR. That's also why I said that implicitly, the key rolling requires the fix for JSON parsing.

arcus-automation commented 4 years ago

A new preview package for Arcus.EventGrid.All is available on MyGet.

You can pull it locally via the CLI:

PM> Install-Package Arcus.EventGrid.All -Version 20200221.0.0-PR-112 -Source https://www.myget.org/F/arcus/api/v3/index.json
tomkerkhove commented 4 years ago

Unassigning as it still looks in-flight

stijnmoreels commented 4 years ago

Is the current state of the PR now something you can agree upon, @tomkerkhove ?

arcus-automation commented 4 years ago

A new preview package for Arcus.EventGrid.All is available on MyGet.

You can pull it locally via the CLI:

PM> Install-Package Arcus.EventGrid.All -Version 20200226.0.0-PR-112 -Source https://www.myget.org/F/arcus/api/v3/index.json
arcus-automation commented 4 years ago

A new preview package for Arcus.EventGrid.All is available on MyGet.

You can pull it locally via the CLI:

PM> Install-Package Arcus.EventGrid.All -Version 20200227.0.0-PR-112 -Source https://www.myget.org/F/arcus/api/v3/index.json