8000 migrate Kinesis to ASF by alexrashed · Pull Request #6166 · localstack/localstack · GitHub
[go: up one dir, main page]

Skip to content

migrate Kinesis to ASF #6166

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

Merged
merged 16 commits into from
Aug 30, 2022
Merged

migrate Kinesis to ASF #6166

merged 16 commits into from
Aug 30, 2022

Conversation

alexrashed
Copy link
Member
@alexrashed alexrashed commented May 30, 2022

This PR migrates Kinesis to ASF. It uses the new streaming support in ASF introduced with #6086 for SubscribeToShard.

In addition, the ASF infrastructure needed to be enhanced with a limited support for HTTP Content-Negotiation in some serializers. For example, the JSON protocols can switch their content to CBOR if requested in the client's request.
Since we now implement more features than botocore's or moto's marshalling, this PR also needed to change the proxy handling to moto and other HTTP backends.
Now, all responses which have been forwarded to moto or any other backend using the fallback mechanism (i.e. the backend implements a specific ASF operation / API) are parsed (using botocore's parsers) and re-serialized (using our - enhanced - serializer).

An obvious downside of this approach is the performance: We have an additional marshalling roundtrip for the responses coming from a fallback backend.
However, in order to properly implement the Content-Negotiation, there isn't really an alternative because we need to support additional features compared to our backends (like moto) as well as boto (which is used to serialize the requests to the fallback backend).

This PR also removes three AwsApiListener extensions (addresses #6668):

  • KinesisApiListener (primary goal of the migration / refactoring)
  • StsAwsApiListener (was also implementing a Content-Negotiation, but for the query protocol)
  • LogsAwsApiListener (useless, the serializer already added the correct content type header)
  • StepFunctionsApiListener (only glue between provider and fallback dispatcher)

TODO:

  • Cleanup code
  • Add tests for new serializer features
  • Flip feature flag before merging into master (the ASF provider is directly becoming the default with the next minor version)
  • Migrate the fix from Kinesis: SubscribeToShard session cut after 5min #6732 to the ASF provider
  • Remove the hypercorn pin / rebase on a fix on master.

@alexrashed alexrashed requested a review from thrau as a code owner May 30, 2022 08:35
@alexrashed alexrashed temporarily deployed to localstack-ext-tests May 30, 2022 08:35 Inactive
@github-actions
Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@github-actions
Copy link
github-actions bot commented May 30, 2022

LocalStack integration with Pro

       3 files  ±0         3 suites  ±0   1h 21m 28s ⏱️ + 2m 40s
1 238 tests  - 1  1 197 ✔️ ±0  41 💤  - 1  0 ±0 
1 663 runs   - 1  1 591 ✔️ ±0  72 💤  - 1  0 ±0 

Results for commit 198b4d4. ± Comparison against base commit 31286eb.

♻️ This comment has been updated with latest results.

@silv-io silv-io removed the size/xl label Jun 13, 2022
@alexrashed alexrashed marked this pull request as draft June 23, 2022 08:38
@alexrashed alexrashed changed the title [WIP] Kinesis ASF Migration migrate Kinesis to ASF Jun 23, 2022
@thrau thrau temporarily deployed to localstack-ext-tests July 27, 2022 12:14 Inactive
@thrau
Copy link
Member
thrau commented Jul 27, 2022

i rebased the branch and added a feature flag/deprecation path to the commit

@thrau thrau temporarily deployed to localstack-ext-tests July 27, 2022 12:25 Inactive
@thrau thrau temporarily deployed to localstack-ext-tests August 16, 2022 13:38 Inactive
@alexrashed alexrashed temporarily deployed to localstack-ext-tests August 16, 2022 14:38 Inactive
@alexrashed alexrashed temporarily deployed to localstack-ext-tests August 18, 2022 09:31 Inactive
@alexrashed alexrashed temporarily deployed to localstack-ext-tests August 18, 2022 11:36 Inactive
@alexrashed alexrashed temporarily deployed to localstack-ext-tests August 19, 2022 14:42 Inactive
@alexrashed alexrashed temporarily deployed to localstack-ext-tests August 22, 2022 12:41 Inactive
@alexrashed alexrashed temporarily deployed to localstack-ext-tests August 22, 2022 13:59 Inactive
@alexrashed alexrashed force-pushed the asf-kinesis branch 2 times, most recently from e82f5e5 to 98b196e Compare August 22, 2022 14:34
@alexrashed alexrashed temporarily deployed to localstack-ext-tests August 22, 2022 14:34 Inactive
@coveralls
Copy link
coveralls commented Aug 22, 2022

Coverage Status

Coverage decreased (-0.2%) to 91.402% when pulling 198b4d4 on asf-kinesis into 31286eb on master.

@alexrashed alexrashed temporarily deployed to localstack-ext-tests August 29, 2022 19:17 Inactive
@alexrashed alexrashed temporarily deployed to localstack-ext-tests August 30, 2022 12:24 Inactive
@alexrashed alexrashed marked this pull request as ready for review August 30, 2022 12:26
Copy link
Member
@thrau thrau left a comment

Choose a reason for hiding this comment

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

As discussed with @alexrashed, we will merge this now to get it into v1.1, and at some later point, should the need arise, refactor the serializer API according to the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0