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 configurable timeouts for REST and gRPC #211

Merged
merged 3 commits into from
Aug 30, 2018
Merged

Add configurable timeouts for REST and gRPC #211

merged 3 commits into from
Aug 30, 2018

Conversation

ukclivecox
Copy link
Contributor

No description provided.

@ukclivecox ukclivecox merged commit 8ba1cef into SeldonIO:master Aug 30, 2018
agrski added a commit that referenced this pull request Dec 2, 2022
* Add structs representing events in MixPanel format

* Add test cases for parsing events in MixPanel format from JSON

* Add custom unmarshaller for event properties

The properties are a combination of expected fields and arbitrary additional ones.
We could simply use a map, but that does not adequately represent the expected fields,
making calling code more tedious with existence checks.

* Add recorder/store for incoming events

These events are recorded in order of incidence.

* Make recorder thread-safe with a write lock

* Add (WIP) HTTP server compatible with MixPanel's Track API

* Return 400 status code for unparsable events

* Join simple lines in handler logic

* Add no-op recorder for testing, decorators, etc.

* Add recorder decorator counting number of events seen

* Use more idiomatic REST error handling

* Add basic cmd package for stub receiver

Here, 'basic' means with hard-coded values.

* Refactor Make build targets to cmd-specific & all Hodometer apps

* Add constructor for no-op recorder

* Allow both GET & POST requests in stub receiver

* Refactor event decoding to method in stub receiver

* Refactor Status response handling to method in stub receiver

* Simplify status-handling method in stub receiver

Idiomatically, error handling and the 'uncommon' case should be nested,
whereas the happy path should remain as unnested as possible.

* Remove superfluous setting of OK response code in stub receiver

* Add CLI args for stub receiver

* Add support for different recording levels

* Add source & func fields to logger in stub receiver

* Add compile-time interface implementation checks

* Add event details method to Recorder interface

Add method to interface.
Add method to implementations.
Change write-only to read-write locks for improved concurrency (assuming frequent reads).

* Implement handler for event details in stub receiver

* Add custom JSON marshalling for event properties

Whilst we internally separate well-known fields from arbitrary ones,
the law of least surprise dictates that events are returned in the
flattened structure they were provided in.

* Return type error on failing to parse insert ID from JSON

* Remove superfluous unmarshalling case for JSON tokens

* Add test case for unmarshalling incomplete data

* Refactor well-known property keys to package constants in stub receiver

* Fix incorrect expected error in unmarshalling test case

Also reorder test cases so those expecting an error are grouped together.

* Fail unmarshalling if any required property are not present

* Add expected errors in unmarshalling test cases

* Add insert ID to Hodometer events

The insert ID is the creation timestamp of the event.
This event can then be retried, even if the timestamp of the event were to change.
This allows for retries of the HTTP request; subsequent metrics publishing will
produce a new timestamp and thus a new logical event.

* Add test for custom Event marshalling into JSON

* Add extra tests cases for event marshalling into JSON

* Use recursive discovery of test files

Previously, only tests at the top level were discovered.  This was a bug.

* Support custom metrics publishing endpoint

This allows for testing with the stub receiver.
It is not intended for users to override, as the API key is deliberately hard-coded.

* Refactor default log level to package-level constant

This aids both consistency and clarity.

* Refactor arg parsing logic into per-arg functions

The existing function was growing long enough to become difficult to read.

* Refactor arg-parsing error logging to main function

It felt weird to have the arg-handling logic accept a logger, but then return configuration
for logging.
It was not clear whether this should be allowed to override the log level for outputting errors or not.
Now, the main function is fully responsible for how it wants to configure the logger.

* Support publishing metrics to multiple endpoints

* Publish to all API endpoints concurrently

As there are retries enabled, it could take minutes for a single endpoint to be published to,
or to fail.
There is no point in holding up the other publishing calls for such cases.

* Rename Dockerfile to disambiguate from stub receiver & update Makefile

* Build only hodometer (not stub receiver) within its Dockerfile

* Set non-root user in hodometer Dockerfile

Also add build args to allow setting the user and group IDs.

* Add Dockerfile for hodometer stub receiver

* Move hodometer files to sibling dir of stub receiver

This separation allows independent testing of both submodules.

* Update hodometer package references in cmd to reflect new package path

* Move hodometer cmd package to sibling dir of receiver cmd package

* Update path for hodometer build in Make target

* Separate testing Make targets for hodometer & stub receiver

* Add Docker Make target for stub receiver

* Use regexes to avoid flakiness in checking error messages

The error message for unmarshalling is constructed using a map,
the keys of which are not guaranteed to be in any particular order,
thus any set order of missing fields in a test could be wrong for the actual result.

* Add test for receiver /track handler

* Add handling for empty request bodies in /track endpoint

This allows differentiation of no event vs. an incomplete or ill-formatted event.

* Add test cases for ill-formatted & incomplete events for receiver handler

* Use nil event when error expected in test

* Add more event-parsing test cases for missing data

* Rename test case for consistency

* Add custom Event unmarshalling method for stub receiver

It is cleaner to fail on missing fields whilst unmarshalling the payload,
rather than performing post-hoc checks.
This ensures the checks will be performed, and is within a well-known interface
for parsing/unmarshalling logic.

* Add verbose flag for testing Make targets

* Add happy-path test cases for /track handler in stub receiver

This includes both the verbose and non-verbose versions of the happy path,
as well as both the request body and query parameter approaches to passing
event data.
In addition, rename test fields for clarity and add additional checks on 'status' responses.

* Join short per-param lines in receiver test cases for concision

* Use POST HTTP method when setting request body in receiver tests

* Rename test param for semantic clarity

The field is an error message, not actually an error.

* Update package path for build/version info in Makefile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant