Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a configurable /v1/events-test route #9

Merged
merged 3 commits into from
Aug 5, 2020

Conversation

ottomata
Copy link
Contributor

@ottomata ottomata commented Aug 5, 2020

No description provided.

If `test_events` is configured, a `GET /v1/events-test` route will be added.
When requested, the `test_events` will be produced as if they were POSTed to
/v1/events. This is useful for readiness probes that want to make sure the
service can produce events end to end.

Bug: T251935
@ottomata ottomata requested a review from Pchelolo August 5, 2020 19:18
README.md Outdated
@@ -231,6 +231,12 @@ services:
# ...
```

# /v1/events-test route
If you are using EventGate as a service, and if `test_events` is configured,
a `GET /v1/events-test` route will be added. When requested, the `test_events` will be produced as if
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Should it be a POST route requiring no body? having GET produce things doesn't seem safe...
  2. should it be prefixed with _ since it's an internal thingy? or, even not prefixed with v1 but something else? like _internal/v1/
  3. should it maybe be called in a more generic way? like /_internal/v1/probe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Has to be GET, I want to use this with a k8s readinessProbe which does not support POST.
  2. & 3. Hm. not sure. Let's discuss in IRC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going with /v1/_test/events

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it!

More clearly indicates that this is a route for internal use.
@Pchelolo Pchelolo self-requested a review August 5, 2020 19:46
@ottomata ottomata merged commit d3326ba into wikimedia:master Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants