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 experimental event ruler #10615

Merged
merged 34 commits into from Apr 18, 2024
Merged

Add experimental event ruler #10615

merged 34 commits into from Apr 18, 2024

Conversation

joe4dev
Copy link
Member

@joe4dev joe4dev commented Apr 8, 2024

Motivation

We have several parity gaps and bugs in multiple different implementations for event pattern matching (see PR #10599). We can offer high AWS-parity quickly using the Amazon EventBridge Event Bus event matching now runs on Open Source Event Ruler v1.5.0 released event-ruler Java library from AWS described in this blog post.

Changes

  • Add an experimental opt-in EVENT_RULE_ENGINE=java flag using the Java event-ruler library from AWS through JPype.
  • Add a MavenPackageInstaller utility (could probably move to localstack/packages/core.py DONE)

Testing

  1. Set EVENT_RULE_ENGINE=java
  2. Ensure that JAVA_HOME points to a valid JVM installation if you are testing in host mode
  3. Uncomment all SKIP_LABELS for the test test_test_event_pattern
  4. Run the test test_test_event_pattern

LocalStack Shutdown

@dfangl and @joe4dev tested the LocalStack shutdown as follows:

  1. Build LS image using make docker-build
  2. Start LocalStack using EVENT_RULE_ENGINE=java DEBUG=1 localstack start
  3. Run a test case for event pattern matching (e.g., `tests.aws.services.events.test_event_patterns.test_test_event_pattern_with_escape_characters) against the running LS instance
  4. docker stop $CONTAINER_ID should stop the container immediately and not after a 10s delay

=> worked without delay

Limitations

  • The cross-language call from Python to Java is fast but could add a few milliseconds of delay.
  • The tight coupling between Python and Java sharing the same process could crash each others language runtime upon unexpected errors.
  • We had to disable the JVM shutdown because LocalStack has too many daemon threads running and JPype only triggers the shutdown after all daemon threads terminated to prevent shutting down the JVM while potentially still in use.
  • The arm build failed with ERROR: Could not build wheels for jpype1 due to gcc: fatal error: cannot execute ‘cc1plus’: execvp: No such file or directory. Supposedly, JPype only supports up to Python 3.10 🤔, which would be a blocker 🔴 (they recently merged a fix for Python 3.12 though). We are using Python 3.11 in the image and jpype1 builds fine on my ARM MacBook using Python 3.11.7.
    • This build error only happened for ARM builds on Linux and required the extra dependency apt-get install -y g++.
    • For S3, we had to add this installation step into the make target docker-run-tests-s3-only to keep it out of the S3 image.

Kinesisanalytics

Kinesisanalytics seems incompatible with EVENT_RULE_ENGINE=java indicated by the following failing tests:

  • TestKinesisAnalyticsProvider.test_tag_list_tag_untag_resource
  • TestKinesisAnalyticsProvider.test_input_processing_configuration
  • TestKinesisAnalyticsProvider.test_application_output
  • TestKinesisAnalytics.test_run_siddhi_query

⇒ all these tests keep failing in the AMD64 pipeline. They are not ARM compatible.
⇒ They fail with the assertion: AssertionError: assert 'STARTING' in ['READY', 'RUNNING']
⇒ The siddhi query test also fails with a name error NameError: name 't_siddhi_api_core' is not defined in the Siddhi loader PySiddhi/SiddhiLoader.py

LS uses some badly outdated fork of the unmaintained (5y; LS 3y last commit) PySiddhi project: https://github.com/localstack/PySiddhi. They are using some kind of custom Python⇒Java bridge based on JNI, which could cause interference.

ℹ️ We decided to ignore this limitation for now. If JPype proves successful, it might be a better alternative than PySiddhi ...

TODO

Follow Up

  • Fix LS Python-native implementation for tests.aws.services.lambda_.test_lambda_integration_dynamodbstreams.TestDynamoDBEventSourceMapping.test_dynamodb_event_filter
    • numeric_filter fails due to missing anything-but support
    • exists_false_filter fails due to broken implementation
    • => It is likely better to adopt Ben's SNS implementation and unify them all because the current status is hardly maintainable.

@joe4dev joe4dev added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Apr 8, 2024
@joe4dev joe4dev self-assigned this Apr 8, 2024
@joe4dev joe4dev force-pushed the add-experimental-event-ruler branch from cd43cbd to a67c3c6 Compare April 9, 2024 14:07
Copy link

github-actions bot commented Apr 9, 2024

S3 Image Test Results (AMD64 / ARM64)

  2 files  ±0    2 suites  ±0   2m 56s ⏱️ -8s
398 tests ±0  346 ✅ ±0   52 💤 ±0  0 ❌ ±0 
796 runs  ±0  692 ✅ ±0  104 💤 ±0  0 ❌ ±0 

Results for commit dc265aa. ± Comparison against base commit 4b72d6e.

♻️ This comment has been updated with latest results.

@joe4dev joe4dev force-pushed the add-experimental-event-ruler branch from 58a4bd4 to 017bae5 Compare April 11, 2024 21:24
Copy link

github-actions bot commented Apr 11, 2024

LocalStack Community integration with Pro

    2 files  ± 0      2 suites  ±0   1h 30m 58s ⏱️ -12s
2 882 tests +32  2 598 ✅ +32  284 💤 ±0  0 ❌ ±0 
2 884 runs  +32  2 598 ✅ +32  286 💤 ±0  0 ❌ ±0 

Results for commit dc265aa. ± Comparison against base commit 4b72d6e.

This pull request removes 35 and adds 67 tests. Note that renamed tests count towards both.
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApi ‑ test_authorizer_crud_no_api
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApi ‑ test_create_proxy_resource
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApi ‑ test_create_proxy_resource_validation
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApi ‑ test_create_resource_parent_invalid
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApi ‑ test_create_rest_api_with_custom_id_tag
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApi ‑ test_create_rest_api_with_optional_params
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApi ‑ test_create_rest_api_with_tags
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApi ‑ test_delete_resource
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApi ‑ test_doc_arts_crud_no_api
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApi ‑ test_get_api_case_insensitive
…
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiAuthorizer ‑ test_authorizer_crud_no_api
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiDocumentationPart ‑ test_doc_parts_crud_no_api
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiMethod ‑ test_method_lifecycle
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiMethod ‑ test_method_request_parameters
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiMethod ‑ test_put_method_model
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiMethod ‑ test_put_method_validation
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiMethod ‑ test_update_method
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiMethod ‑ test_update_method_validation
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiModels ‑ test_model_lifecycle
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApiModels ‑ test_model_validation
…
This pull request removes 1 skipped test and adds 1 skipped test. Note that renamed tests count towards both.
tests.aws.services.apigateway.test_apigateway_api.TestApiGatewayApi ‑ test_get_api_case_insensitive
tests.aws.services.events.test_event_patterns ‑ test_test_event_pattern[exists_dynamodb_NEG]

♻️ This comment has been updated with latest results.

# Determine which implementation to use for the event rule / event filtering engine used by multiple services:
# EventBridge, EventBridge Pipes, Lambda Event Source Mapping, SNS
# Options: provider (default) | java
EVENT_RULE_ENGINE = os.environ.get("EVENT_RULE_ENGINE", "").strip()
Copy link
Member Author

Choose a reason for hiding this comment

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

✅ CI run with JPype enabled using java here in 017bae5

# Workaround to unblock LocalStack shutdown. By default, JPype wait until all daemon threads are terminated,
# which blocks the LocalStack shutdown upon any leaked thread (know LS issue).
# See https://github.com/MPh-py/MPh/issues/15#issuecomment-778486669
jpype_config.destroy_jvm = False
Copy link
Member Author

Choose a reason for hiding this comment

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

That's unfortunately needed. Could this cause any side-effects?

Copy link
Member

Choose a reason for hiding this comment

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

The container shutdown will probably not work, as we have an orphan process which did not get terminated by its parent on shutdown. The supervisor will probably wait forever, until docker kills the container.

pyproject.toml Outdated
@@ -67,6 +67,8 @@ base-runtime = [
"Werkzeug>=3.0.0",
"xmltodict>=0.13.0",
"rolo>=0.4",
# allow Python programs full access to Java class libraries. Used for opt-in event ruler.
"JPype1>=1.5.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

431KB without any extra dependencies (see their pyproject.toml), just packaging and typing_extensions, which we have as well

Copy link
Contributor

@bentsku bentsku Apr 17, 2024

Choose a reason for hiding this comment

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

could we move it to runtime instead? base-runtime is the base one used by the S3 image, which has no dependency basically. Or was there a specific reason to put into base-runtime?

For a bit of context, basically, base-runtime is the minimal set of dependencies needed to run a service in LocalStack which would be entirely written in Python with no external dependencies (like S3).
I believe this change, which is for a provider with external dependencies and only opt-in, would fit better in runtime.

This would also have the side-effect of not having to change the S3 image Dockerfile, which does not need this and g++.

Copy link
Member Author

Choose a reason for hiding this comment

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

That makes a lot of sense @bentsku 💯 Thank you for this valuable input 👀 👏
I fixed everything (including the follow-up S3 tests because the test deps include the runtime deps 😬 ; that's a bit of an ugly hack though)

I didn't understand the full implications of base-runtime vs runtime dependencies.

With the fixed solution, the S3 image does not get larger 👌

# {"id": {"S": "id_value_1"}},
# {"id": {"S": "id_value_2"}, "presentKey": {"S": "presentValue"}},
# {"dynamodb": {"NewImage": {"presentKey": [{"exists": False}]}}},
# 2,
Copy link
Member Author

Choose a reason for hiding this comment

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

This false negative took me forever to spot. A combination of bad test design and inappropriate configuration led to wrong assumptions!
It was configured to 1 but both events match against AWS. However, the second event never gets waited. Negative testing for the last event is unreliable!

Copy link
Member

Choose a reason for hiding this comment

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

Will this test be reintroduced at a later point?

function_name = f"lambda_func-{short_uid()}"
table_name = f"test-table-{short_uid()}"
max_retries = 50

create_lambda_function(
handler_file=TEST_LAMBDA_PYTHON_ECHO,
func_name=function_name,
runtime=Runtime.python3_9,
runtime=Runtime.python3_12,
Copy link
Member Author

Choose a reason for hiding this comment

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

/cc @Morijarti I'm bumping the runtime here together with significant test changes, so can can simply take over my changes upon rebasing

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, thank you

@@ -136,7 +136,7 @@ def test_no_match_partial(self):
filters = [
{
"Filters": [
{"Pattern": json.dumps({"partitionKey": "2", "data": {"City": ["Seattle"]}})}
{"Pattern": json.dumps({"partitionKey": ["2"], "data": {"City": ["Seattle"]}})}
Copy link
Member Author

Choose a reason for hiding this comment

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

another example why non-validated unit-tests are tricky to get right 😢

Dockerfile.s3 Outdated
apt-get install -y gcc
apt-get install -y gcc && \
# Workaround to fix JPype1 compile error on ARM Linux "gcc: fatal error: cannot execute ‘cc1plus’"
apt-get install -y g++
Copy link
Member Author

Choose a reason for hiding this comment

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

I wished this was not necessary, especially not needed for S3 😿

Copy link
Member

Choose a reason for hiding this comment

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

I guess this would be a bit faster if it would be in a single apt-get install call? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

And maybe this should be added somewhere in the contribution guide? Because it seems Mac users will struggle to run make install without g++ (and Java when they use this feature) on their machines?

Copy link
Member Author

Choose a reason for hiding this comment

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

+1 for a single apt-get call, updated ✅

The g++ dependency might be specific to ARM Linux because it worked out of the box for me on an ARM MacBook.

I agree that the Java installation and setting JAVA_HOME is a bit cumbersome in host mode 😿
@alexrashed Where is the right place to document service-specific requirements for host mode? (we have Notion, the dev guide on the website, getting started guide)

Copy link
Member

Choose a reason for hiding this comment

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

I think this should be part of the contribution guidelines (because it affects all open source contributors), which is currently located in our docs.

Copy link
Contributor

@bentsku bentsku Apr 17, 2024

Choose a reason for hiding this comment

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

Why do we need it for S3? Because we put it into base-runtime? Could we just move the dependency into runtime instead? I'm not sure I entirely follow here.
(I probably should add the S3 docker file into the code owners list for S3 as well, I totally missed this PR 😅)

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't need it for S3. Thanks for suggesting the switch to runtime deps 👍
Fixed as commented in #10615 (comment)

@joe4dev joe4dev marked this pull request as ready for review April 12, 2024 08:35
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Wow, this a pretty sophisticated change which drastically improve parity with the event pattern matching! 💯
I hope the new dependency doesn't cause too many issues with #10235 (but as you stated, they seem to work on compatibility above 3.10).
Besides that I only had some thoughts concerning the internal developer experience (Mac users need to be aware that they have to install g++) and some smaller nitpicks ;)
Besides that, I'll leave the final signoff to the service owners :)

localstack/services/events/packages.py Outdated Show resolved Hide resolved
localstack/services/events/packages.py Outdated Show resolved Hide resolved
@joe4dev joe4dev force-pushed the add-experimental-event-ruler branch from 0aca7a1 to ec960ea Compare April 15, 2024 11:00
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Awesome! 🚀 Thanks a lot for addressing all the comments! From my side all these changes are looking good! As mentioned in the initial comment, I'll leave the final sign-off to the code owners 🥳

localstack/services/events/provider.py Outdated Show resolved Hide resolved
# Workaround to unblock LocalStack shutdown. By default, JPype wait until all daemon threads are terminated,
# which blocks the LocalStack shutdown upon any leaked thread (know LS issue).
# See https://github.com/MPh-py/MPh/issues/15#issuecomment-778486669
jpype_config.destroy_jvm = False
Copy link
Member

Choose a reason for hiding this comment

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

The container shutdown will probably not work, as we have an orphan process which did not get terminated by its parent on shutdown. The supervisor will probably wait forever, until docker kills the container.

localstack/services/events/event_ruler.py Outdated Show resolved Hide resolved
Comment on lines 14 to 20
if config.EVENT_RULE_ENGINE == "java":
import jpype
import jpype.imports
from jpype import config as jpype_config
from jpype import java

Copy link
Member

Choose a reason for hiding this comment

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

Is it necessary to do this on module level on import? including the startup? Could it be done on first match as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess we can go full dynamic import upon first event matching:

  • it could hide potential issues
  • JVM startup delay is probably not an issue though.

Do you think a lazy import is better here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Resolved using a dynamic import now (as discussed in sync).

# {"id": {"S": "id_value_1"}},
# {"id": {"S": "id_value_2"}, "presentKey": {"S": "presentValue"}},
# {"dynamodb": {"NewImage": {"presentKey": [{"exists": False}]}}},
# 2,
Copy link
Member

Choose a reason for hiding this comment

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

Will this test be reintroduced at a later point?

@joe4dev
Copy link
Member Author

joe4dev commented Apr 15, 2024

Trying to reply to @dfangl comment: #10615 (comment)

The container shutdown will probably not work, as we have an orphan process which did not get terminated by its parent on shutdown. The supervisor will probably wait forever, until docker kills the container.

This a problem ⚠️ 😢

How should we test this? Any suggestion on how to fix this?

@joe4dev joe4dev force-pushed the add-experimental-event-ruler branch from efaf60c to 9a54628 Compare April 16, 2024 10:22
@joe4dev
Copy link
Member Author

joe4dev commented Apr 16, 2024

Trying to reply to @dfangl comment: #10615 (comment)

The container shutdown will probably not work, as we have an orphan process which did not get terminated by its parent on shutdown. The supervisor will probably wait forever, until docker kills the container.

This a problem ⚠️ 😢

How should we test this? Any suggestion on how to fix this?

This should be resolved after successfully testing the shutdown behavior together with @dfangl (in sync today)

The CI is looking good here 🟢 . In ext, we have some Kinesis analytics test still failing (e.g., TestKinesisAnalyticsProvider.test_application_output) ....

@joe4dev
Copy link
Member Author

joe4dev commented Apr 17, 2024

Using EVENT_RULE_ENGINE=java, all tests are 🟢 in community. However, in ext, we have some Kinesis analytics test still failing (e.g., TestKinesisAnalyticsProvider.test_application_output).

The failing Kinesisanalytics tests are likely caused by an interaction with our Kinesisanalytics implementation using https://github.com/localstack/PySiddhi, an unmaintained project that uses some kind of custom Python=>Java bridge. This C-based proxy uses JNI (like JPype) and could potentially cause an unintended interaction.

Failing ext test run:

  • TestKinesisAnalyticsProvider.test_tag_list_tag_untag_resource
  • TestKinesisAnalyticsProvider.test_input_processing_configuration
  • TestKinesisAnalyticsProvider.test_application_output
  • TestKinesisAnalytics.test_run_siddhi_query

@joe4dev
Copy link
Member Author

joe4dev commented Apr 17, 2024

Without using the new opt-in Java-based event-ruler, all tests are green 🟢 , both in community and ext 🚀

Copy link
Member

@dfangl dfangl left a comment

Choose a reason for hiding this comment

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

Looks good now, thank you for addressing my comments and avoiding start of the jvm on module import.
Looking forward for more unification of all the event matching implementations :)

@andy-g
Copy link

andy-g commented Apr 17, 2024

Could this also be supported when running localstack in a docker container? And I assume this will resolve the current issues with what appears to be unsupported syntax such as:

There are current matching rules that don't appear to be working for me, I'm currently running localstack 3.3 in docker.

@joe4dev
Copy link
Member Author

joe4dev commented Apr 17, 2024

Thanks for the comment @andy-g

Could this also be supported when running localstack in a docker container?

Yes, it will work in Docker.

And I assume this will resolve the current issues with what appears to be unsupported syntax such as:
Correct, I assume EVENT_RULE_ENGINE=java should fix all issues related to event pattern matching for EventBridge, EventBridge Pipes, and Lambda Event Source Mapping. It should reach almost 100% AWS parity. We currently do not yet implement the same complexity calculation, so very complex pattern might work in LocalStack but throw an exception in AWS.
We are also looking into how to adopt the same rule engine for SNS (which already implements some complexity calculations). //cc @bentsku

All builds are 🟢 , so I expect this to be merged anytime soon 🚀

@andy-g
Copy link

andy-g commented Apr 17, 2024

All builds are 🟢 , so I expect this to be merged anytime soon 🚀

Brilliant, thanks @joe4dev, I'll keep an eye on this PR then, and thanks so much for the quick response.

@maxhoheiser
Copy link
Member

Very nice PR, I expect to use this heavily in our new native event bridge implementation. Although tests with this java pattern matching take a bit longer to execute for me compared to our own implementation - it is much more complete. I also expect it to perform much better once a certain event number is put through the system at the same time. This will come in handy if we can get the rest of the provider's performance also up that level of speed.

@joe4dev joe4dev added this to the 3.4 milestone Apr 17, 2024
Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

I've added 2 comments: #10615 (comment) and #10615 (comment), which I believe should be addressed before merging if possible, as this PR would then have no impact on the S3 image or base runtime. There might be something I missed, feel free to correct me if it's really needed to go into base-runtime. Thanks!

Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

thanks a lot for removing the changes for the S3 image, sorry it has been a bit complicated with the tests dependencies and had to go further down the path of quick hacks in the make target 😬 but very grateful you've addressed it. Thank you so much!

And sorry again for missing this and coming in late requesting changes. 😞

Great work, can't wait for us to have a unified event ruler! 🚀

@joe4dev joe4dev merged commit d858f00 into master Apr 18, 2024
41 checks passed
@joe4dev joe4dev deleted the add-experimental-event-ruler branch April 18, 2024 11:01
@andy-g
Copy link

andy-g commented Apr 18, 2024

@joe4dev I see this was merged, and I see that new docker images have been built, am I right in assuming these changes should be in latest?

I'm running:

docker pull localstack/localstack:latest
latest: Pulling from localstack/localstack
Digest: sha256:e9b46037d6af0b947aad43895edf371b9de96b17cacb52383f5c6c7a2467817a
Status: Image is up to date for localstack/localstack:latest
docker.io/localstack/localstack:latest

Using this rule my lambda works (as it has previously):

{
  "detail-type": ["PaymentReceived","PaymentCompleted"],
  "source": ["producer-1"]
}

...but changing my rule to either of these result in the lambda not being invoked (I was hoping this PR would change that):

{
  "detail-type": ["PaymentReceived"],
  "detail.payload.paymentDetails": {
    "url": [{"exists": true}],
    "type": ["Multi"]
  },
  "source": ["producer-1"]
}
{
  "$or": [
    {
      "detail-type": ["PaymentCompleted"],
      "detail.payload.url": [{"exists": true}]
    },
    {
      "detail-type": ["PaymentReceived"],
      "detail.payload.paymentDetails": {
        "url": [{"exists": true}],
        "type": ["Multi"]
      }
    }
  ],
  "source": ["producer-1"]
}

These are working fine on AWS itself.

Am I missing something here?

@andy-g
Copy link

andy-g commented Apr 18, 2024

Sorry, after a closer look I realised I needed to set the EVENT_RULE_ENGINE environment variable to java. I added that to my docker-compose environment array and it's working like a charm, thanks so much 🍻 .

@joe4dev
Copy link
Member Author

joe4dev commented Apr 18, 2024

I see this was merged, and I see that new docker images have been built, am I right in assuming these changes should be in latest?

Yes, the changes are released for localstack/localstack but NOT yet for localstack/localstack-pro (I just merged a fix for a hidden ARM build issue in our CI, hence the pro image should be released in the next few hours 🤞 ).

@andy-g Awesome to hear everything works as expected using EVENT_RULE_ENGINE=java 🚀 🙌
We wanted to learn more how the Python=>Java bridge works before rolling out that feature by default. That's why we opted for a feature flag. I'm looking forward to hearing your feedback if you notice anything (e.g., performance, stability etc) 👂

@joe4dev
Copy link
Member Author

joe4dev commented Apr 18, 2024

@andy-g I can confirm the localstack/localstack-pro image is now successfully released and the new opt-in event-ruler is working 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants