temporalio / roadrunner-temporal

Temporal PHP-SDK Host Process plugin for Roadrunner
MIT License
22 stars 8 forks source link

feat: upsert search attributes #275

Closed cv65kr closed 2 years ago

cv65kr commented 2 years ago

Reason for This PR

https://github.com/temporalio/sdk-php/issues/76

Same functionality like in GO-SDK - https://github.com/temporalio/samples-go/tree/main/searchattributes

Changes needed in PHP-SDK - https://github.com/temporalio/sdk-php/pull/248

Description of Changes

Added missing implementation of UpsertWorkflowSearchAttributes command.

E2E

Changes: https://github.com/roadrunner-server/rr-e2e-tests/pull/138

Workflow file: https://github.com/temporalio/sdk-php/blob/6a40e81ba7125ee32fbb242599f65ddff649500a/tests/Fixtures/src/Workflow/UpsertSearchAttributesWorkflow.php

Also definition of search attributes needs to be executed to test server before running workflow - https://github.com/temporalio/sdk-php/blob/6a40e81ba7125ee32fbb242599f65ddff649500a/tests/SearchAttributeTestInvoker.php

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

PR Checklist

[Author TODO: Meet these criteria.] [Reviewer TODO: Verify that these criteria are met. Request changes if not]

rustatian commented 2 years ago

Hey @cv65kr 👋🏻 Please, do not edit the PR template.

cv65kr commented 2 years ago

@rustatian Updated, also signed my commits. And big sorry for removing template 😄

I will back to you as soon as I test whole implementation together with SDK-PHP.

cv65kr commented 2 years ago

@rustatian fixed and tested. I did small mistake and encoder was not needed.

image
rustatian commented 2 years ago

@cv65kr Could you please add a PHP worker to the description (which sends this command)? I'll add it to our international tests: https://github.com/roadrunner-server/rr-e2e-tests.

cv65kr commented 2 years ago

@rustatian You meant workflow definition? I see general worker https://github.com/roadrunner-server/rr-e2e-tests/blob/master/php_test_files/temporal-worker.php

Also added info about definition of search attributes which need to be executed before running such test.

cv65kr commented 2 years ago

@rustatian to make your live easier I prepared changes in E2E repository - https://github.com/roadrunner-server/rr-e2e-tests/pull/138 Feel free to modify it since I put my branches there, because we need to wait for merge.

rustatian commented 2 years ago

Great, thank you, @cv65kr 👍🏻 I'll review and merge it tomorrow ⚡