Add Event-Gateway component #182

marcobebway · 2018-08-13T12:49:22Z

Confirm these statements before you submit your pull request:

I have read and submitted the required Contributor Licence Agreements.
This pull request follows the contributing guidelines, recommended Git workflow, and templates.
I have tested my changes.
I have updated the relevant documentation.

Description

Changes proposed in this pull request:

add event-gateway component inside kyma
add event-gateway entry to the CODEOWNERS file
add event-gateway entry to the orchestrator.Jenkins file

Issue link
#405

klaudiagrz · 2018-08-13T13:25:54Z

components/event-gateway/README.md

+
+The Event-Gateway has the following parameters:
+- **externalAPIPort** - This port exposes the Event-Gateway API to an external solution. The default port is `8081`.
+- **eventsTargetURL** - A URL to which you proxy the incoming Events. The default URL is http://localhost:9000.


The default URL is `http://localhost:9000`.

klaudiagrz · 2018-08-13T13:29:26Z

What is the relation of this PR to #166 ?

marcobebway · 2018-08-13T13:56:18Z

@klaudiagrz I split the PR #166 into two PRs:

PR Add Event-Gateway component #182 to:
- add event-gateway as a new component in kyma
PR Extract the Event service outside of the Gateway service #166 to:
- add event-gateway component helm charts
- remove event handling from gateway codebase
- remove event related config from gateway helm charts
- update kyma gateway acceptance-tests to use event-gateway instead of gateway

Please note that, first we need to merge this PR #182, and then we will work on PR #166 (which is now labeled as WIP).

klaudiagrz · 2018-08-13T13:59:49Z

Ok, thanks for explanation :)

ghost · 2018-08-14T06:47:17Z

components/event-gateway/internal/events/bus/send.go

+
+	reqURL, err := url.ParseRequestURI(eventsTargetURL)
+	if err != nil {
+		fmt.Println(err)


If "err != nil" an error should be returned.
"reqUrl" could be nil

This is a code copy from the gateway component.
However, this is fixed because it is minor.

akgalwas · 2018-08-14T08:17:28Z

components/event-gateway/internal/apperrors/apperrors.go

@@ -0,0 +1,53 @@
+package apperrors


Type apperrors was intruduced but it is referenced in apperrors_test.go

akgalwas · 2018-08-14T08:25:37Z

components/event-gateway/internal/events/bus/process.go

+	"github.com/kyma-project/kyma/components/event-gateway/internal/events/api"
+)
+
+type configurationData struct {


I'm not certain what is the purpose of using pointer here, I think it adds some complexity. In main.go "&" operator is used to pass string's pointer to bus.Init. In bus.Init configurationData members are set but in AddSource * operator is used to dereference the pointer. I think bus.Init and configurationData could use string instead of *string (passing by value is more natural here).

This is a code copy from the gateway component.
Refactoring will be considered separately not in this pull request.

* add event-gateway component inside kyma * add event-gateway entry to the CODEOWNERS file * add event-gateway entry to the orchestrator.Jenkins file

akgalwas · 2018-08-14T08:40:12Z

components/event-gateway/internal/events/bus/send.go

+}
+
+// SendEvent sends the incoming request to the Sender
+func SendEvent(req *api.SendEventParameters, traceHeaders *map[string]string) (*api.SendEventResponse, error) {


I would propose to refactor this function a bit. I would extract the following operations into separate functions : creating http request, unmarshaling correct response, unmarshalling error response. Such a change would make SentEvent shorter and easier to read. What do you think?

This is a code copy from the gateway component.
Refactoring will be considered separately not in this pull request.

akgalwas · 2018-08-14T08:49:52Z

components/event-gateway/internal/events/bus/process.go

+	sourceEnvironment string
+}
+
+var conf *configurationData


I would avoid global variables. Consider declaring struct (e.g. EventBus) with configuration and eventTargetURL properties. Init function would be a factory method and return the struct (or an interface if you would like to mock it). What do you think?

This is a code copy from the gateway component.
Refactoring will be considered separately not in this pull request.

marcobebway · 2018-08-14T11:08:44Z

@akgalwas Refactoring will be considered separately not in this pull request.

clebs

LGTM.
Only comment: Remember to add a new job into the kyma seedjob.

marcobebway · 2018-08-16T08:11:54Z

@a-thaler, @derberg, @pbochynski, @PK85 please review.

derberg

I couldn't find any documentation and it is not clear what this new component is all about

marcobebway · 2018-08-16T08:44:20Z

@derberg the documentation is here: components/event-gateway/README.md.

derberg

@marcobebway no this is not product documentation, this is a README of the component. Or are you planning to provide documentation the same time you create a new chart and release it to Kyma?

marcobebway · 2018-08-16T14:21:33Z

Yes @derberg, we will provide product documentation when we create a new chart and release it to Kyma.

marcobebway requested review from a-thaler, clebs and derberg as code owners August 13, 2018 12:49

marcobebway requested a review from a user August 13, 2018 12:49

marcobebway requested review from kazydek, klaudiagrz, lilitgh, pbochynski, PK85, suleymanakbas91 and tomekpapiernik as code owners August 13, 2018 12:49

klaudiagrz reviewed Aug 13, 2018

View reviewed changes

marcobebway force-pushed the add-event-gateway-component branch from a211ae5 to 27167a3 Compare August 13, 2018 13:33

klaudiagrz approved these changes Aug 13, 2018

View reviewed changes

ghost reviewed Aug 14, 2018

View reviewed changes

akgalwas reviewed Aug 14, 2018

View reviewed changes

marcobebway force-pushed the add-event-gateway-component branch from 27167a3 to 743212e Compare August 14, 2018 08:34

Add Event-Gateway component:

3f57fa1

* add event-gateway component inside kyma * add event-gateway entry to the CODEOWNERS file * add event-gateway entry to the orchestrator.Jenkins file

marcobebway force-pushed the add-event-gateway-component branch from 743212e to 3f57fa1 Compare August 14, 2018 08:36

akgalwas reviewed Aug 14, 2018

View reviewed changes

marcobebway mentioned this pull request Aug 14, 2018

Event-Gateway refactoring #192

Closed

2 tasks

clebs approved these changes Aug 14, 2018

View reviewed changes

akgalwas approved these changes Aug 14, 2018

View reviewed changes

ghost self-requested a review August 16, 2018 06:31

ghost approved these changes Aug 16, 2018

View reviewed changes

derberg suggested changes Aug 16, 2018

View reviewed changes

derberg approved these changes Aug 17, 2018

View reviewed changes

marcobebway merged commit e7332eb into kyma-project:master Aug 17, 2018

marcobebway mentioned this pull request Sep 7, 2018

Update helm charts to use new event-service #405

Closed

marcobebway deleted the add-event-gateway-component branch September 26, 2018 19:04

marcobebway restored the add-event-gateway-component branch September 26, 2018 19:04

marcobebway deleted the add-event-gateway-component branch September 26, 2018 19:12

abbi-gaurav mentioned this pull request Aug 31, 2018

Apply refactoring comments for event-service #377

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Event-Gateway component #182

Add Event-Gateway component #182

marcobebway commented Aug 13, 2018 •

edited

klaudiagrz Aug 13, 2018

klaudiagrz commented Aug 13, 2018

marcobebway commented Aug 13, 2018

klaudiagrz commented Aug 13, 2018

ghost Aug 14, 2018

marcobebway Aug 14, 2018

akgalwas Aug 14, 2018

marcobebway Aug 14, 2018

akgalwas Aug 14, 2018

marcobebway Aug 14, 2018 •

edited

akgalwas Aug 14, 2018

marcobebway Aug 14, 2018 •

edited

akgalwas Aug 14, 2018

marcobebway Aug 14, 2018 •

edited

marcobebway commented Aug 14, 2018 •

edited

clebs left a comment

marcobebway commented Aug 16, 2018

derberg left a comment

marcobebway commented Aug 16, 2018 •

edited

derberg left a comment

marcobebway commented Aug 16, 2018 •

edited

Add Event-Gateway component #182

Add Event-Gateway component #182

Conversation

marcobebway commented Aug 13, 2018 • edited

Choose a reason for hiding this comment

klaudiagrz commented Aug 13, 2018

marcobebway commented Aug 13, 2018

klaudiagrz commented Aug 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcobebway Aug 14, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcobebway Aug 14, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcobebway Aug 14, 2018 • edited

Choose a reason for hiding this comment

marcobebway commented Aug 14, 2018 • edited

clebs left a comment

Choose a reason for hiding this comment

marcobebway commented Aug 16, 2018

derberg left a comment

Choose a reason for hiding this comment

marcobebway commented Aug 16, 2018 • edited

derberg left a comment

Choose a reason for hiding this comment

marcobebway commented Aug 16, 2018 • edited

marcobebway commented Aug 13, 2018 •

edited

marcobebway Aug 14, 2018 •

edited

marcobebway Aug 14, 2018 •

edited

marcobebway Aug 14, 2018 •

edited

marcobebway commented Aug 14, 2018 •

edited

marcobebway commented Aug 16, 2018 •

edited

marcobebway commented Aug 16, 2018 •

edited