Skip to content

Conversation

@DeRauk
Copy link
Contributor

@DeRauk DeRauk commented Oct 5, 2021

Right now the error handler is using the global config, we want it to use the config that the client was created with.

@DeRauk DeRauk marked this pull request as draft October 10, 2021 15:39
Copy link
Contributor

@antstorm antstorm 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!

@DeRauk DeRauk marked this pull request as ready for review October 20, 2021 20:39
@DeRauk DeRauk merged commit 3b5a4dc into coinbase:master Oct 20, 2021
@antstorm antstorm added the sync pending Needs to be ported to cadence-ruby label Oct 21, 2021
@DeRauk
Copy link
Contributor Author

DeRauk commented Oct 22, 2021

@antstorm thanks for adding the sync tag 🤦

christopherb-stripe pushed a commit to christopherb-stripe/temporal-ruby that referenced this pull request Jan 9, 2023
…everting-json-proto-changes

reverted proto_json changes
christopherb-stripe pushed a commit to christopherb-stripe/temporal-ruby that referenced this pull request Jan 9, 2023
…/calum/reverting-json-proto-changes"

This reverts commit e805a38, reversing
changes made to 3deda64.
DeRauk pushed a commit that referenced this pull request Apr 7, 2023
* Revert signal_with_start (before taking a different approach)

* Add signal arguments to start_workflow (to support signal_with_start)

* Merge memo changes

* Address PR feedback

* Update method signature in temporal test fixture

* Add detail to a few error messages

* Update our FailWorkflowTask logic's call to ErrorHandler.handle

* Workflow await

* Make dispatch more generic

* Fix race condition

* Merge await into wait_for

* Update sample workflow to use wait_for, rename to WaitForWorkflow

* Reorganize and extend wait_for tests

* Check for completed futures before setting dispatcher and yielding the fiber

* Extend wait_for to take multiple futures and a condition block

* Differentiate TARGET_WILDCARD and WILDCARD, allow comparison with EventTarget objects

* Use Ruby 2.7 to be consistent with pay-server

* Turn off schedule_to_start activity timeout by default

* Refactor metadata generation

* Make task queue available on workflow metadata, add example test

* Expose workflow start time metadata

* Add memos

* Make error deserialization more resilient

* Make temporal-ruby able to deserialize larger histories

* Remove temporary test

* Expose workflow name in activity metadata in temporal-ruby's unit tester

* Add a workflow-level test

* add namespace to emitted metrics

* emit the workflow name tag during activity processing

* fix typo

* added failworkflowtaskerror

* updated register namespace to accept new params

* examples

* fixed namespace test

* empty

* updated unit tests

* removed unncessary code

* updated seconds

* Add replay flag to workflow context

* fixed nits

* Rename to replay? to history_replaying?

* updated sleep to 0.5

* updated unit tests and nits

* fixed unit tests

* added link to comment

* Merge fix

* Fix upsert_search_attributes

* added fix for nil search attributes

* added unit test

* updated unit test

* added expect to be nil

* Expose scheduled_time and current_attempt_scheduled_time on activity metadata

* Implement ParentClosePolicy for child workflows

* Add e2e test for child workflow execution

* move serialization logic farther down the stack

* Refactor serialize_parent_close_policy; add unit tests

* Expose wait_for_start for child workflow execution

* Remove future `workflow_id,run_id` annotations; simplify wait_for logic

* Add parent_run_id, parent_id to workflow metadata

* Allow opting out of child workflow futures

* Remove duplicate describe block

* Factor out workflow_id_reuse_policy serialization

* Respect workflow_id_reuse_policy for child workflows

* Add integration tests

* Use a nicer exception type

* Refactor wait_for into distinct wait_for_any and wait_for_condition methods

* Order wildcard dispatch handlers

* Remove finished handlers

* Check finished? on wait_for_any, add more unit specs

* More dispatch unit specs

* Use hash instead of list for callbacks per target

* Remove dead code, improve error messages in local workflow context

* Correct swapped arguments

* Eliminate unnecessary IDs for dispatcher handlers

* Raise on duplicate ID

* added paginated workflows

* client spec

* reset

* Downstream the rest of [activity_metadata.workflow_name](#130)

* Downstream the rest of [activity_metadata.scheduled_time](#164)

* Fix an activity_metadata related test that doesn't exist upstream

* Downstream the rest of [child workflow workflow_id_reuse_policy fixes](#172)

* Downstream [merge error fix](#180)

* added json protobuf

* Update json_protobuf.rb

* added unit test

* Remove duplicate tests in context_spec

* allow connection options to be set

* add missing comma

* add test for interceptors

* remove unused variable

* use new Temporal client in interceptor test to avoid test pollution

* add option to specify search attributes when starting workflows

* move empty check out of Temporal::Workflow::Context::Helpers.process_search_attributes

* move process_search_attributes out of ExecutionOptions.initialize

* allow default search attributes to be configured globally

* fix tests, unit test global default search attributes

* add unit test for gRPC serialization

* clean up the integration test

* Include remaining changes from upstream #188 (#98)

### Summary

This PR is a follow-up to #97 that includes the changes made to the corresponding upstream PR ([github.com/#188](#188)) after that downstream PR was eagerly merged (with a subset of the changes from upstream). The changes that were not included in the downstream PR when it was merged are: [compare](https://github.com/coinbase/temporal-ruby/pull/188/files/05b6eafeb43ac717c15ae683e6249c4de876ef3d..c6f614325695cc666e066aa218a329c9bf7504f3) (that should be identical to the changes in this PR).

With the merging of the upstream PR, I also updated the [non-upstreamed changed doc](https://paper.dropbox.com/doc/Non-upstreamed-temporal-ruby-changes--Bm39qa99fshz6nbw9RG37pgFAg-AYkwiCfkcoM66adjZMwAZ) to remove the entry on Initial workflow search attributes.

### Motivation
The intention of this PR is to synchronize upstream and downstream, with relation to the changeset applied in the upstream PR.

### Test plan
N/A

### Rollout/monitoring/revert plan
Safe to revert.

* Remove dead code from previous messy merge

* Separate wait_until handlers, execute at end

* Modify signal_with_start_workflow to cover dwillett's repro

* Disable running rubyfmt on save

* Consolidate metrics constants

* Add workflow task failure counter

* Use metric_keys.rb for filename

* Allow client identity to be configurable

* Use PID instead of thread ID in default identity

* Add poller completed metrics

* Gauge metrics for queue size and available threads per thread pool

* Add task queue and namespace to thread pool metrics

* Fix tag

* reverted proto_json changes

* fixed import

* remmoved test file

* Revert "Merge pull request #108 from stripe-private-oss-forks/calum/fixing-import"

This reverts commit 48cbe20, reversing
changes made to e805a38.

* Revert "Merge pull request #107 from stripe-private-oss-forks/calum/reverting-json-proto-changes"

This reverts commit e805a38, reversing
changes made to 3deda64.

* Exempt the necessary system workflows from normal JSON proto deserialization

* Emit task queue for workflow task failures

* Added task queue tags to workflow and activity task queue time metrics

* DynamicActivity

* Use const_get

* Cleanup error message

* Jeff feedback

* do not fail workflow task if completing it errors

It's possible for a workflow task to fail to complete even during normal
operation. For example, if a signal is added to the workflow history
concurrently with a workflow task attempting to complete the workflow
then the `RespondWorkflowTaskCompleted` call will recieve an
`InvalidArgument` error with an `UnhandledCommand` cause.

If this happens then the Ruby SDK would get this error and then try and
fail the workflow task due to how the `rescue` block encompassed the
completion function. This would then lead to the
`RespondWorkflowTaskFailed` call failing because the server doesn't
recognize the task anymore. This is because the task was actually
finalized when the original `RespondWorkflowTaskCompleted` call was made
and so it should not be interacted with anymore.

* add a comment

* use more realistic error type in tests

* Remove orphaned code post-merge

* Allow empty pages when paginating through history

* Remove unused error

* Allow ActivityException to accept args

* Improve deserialization code flow

* Use the default converter to serialize errors when configured to do so

* Get tests working

* Cleanup

* fix nit

* Fix extraneous debug spew

* added max page size param

* Show bad data on Activity error serialization failure

* Upgrade Temporal proto API to version 1.16

* Rename Temporal -> Temporalio

* Remove deprecated namespace field for activity task scheduling

* Methods for operating on custom search attributes

* Example test for custom search attribute APIs

* Unit tests

* Treat missing history submessage on GetWorkflowExecutionHistoryResponse as timeout

* Remove unnecessary &.

* Dynamic Workflows

* Add terminate-if-running workflow id reuse policy mappings

* Add integration test for terminate-if-running

* Move misplaced test

* Feedback

* Use terminate-if-running as policy for both invocations

* added payload codecs

* added search attribute payload methods

* fixed tests

* added codec tests

* added chain tests

* updated nits

* Revert "Merge branch 'master' into calum/adding-payload-codecs"

This reverts commit 145290c, reversing
changes made to d8a42d8.

* fixed tests

* fixed

* examples/lib/crypt_payload_codec.rb

---------

Co-authored-by: Nathan Glass <nagl@stripe.com>
Co-authored-by: Jeff Schoner <jeffschoner@stripe.com>
Co-authored-by: Andrew Hoskins <drewhoskins@stripe.com>
Co-authored-by: Christopher Brown <christopherb@stripe.com>
Co-authored-by: Arya Kumar <aryak@stripe.com>
Co-authored-by: Joseph Azevedo <jazevedo@stripe.com>
Co-authored-by: Jeff Schoner <jeffschoner@gmail.com>
Co-authored-by: Bryton Shoffner <bryton@stripe.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sync pending Needs to be ported to cadence-ruby

Development

Successfully merging this pull request may close these issues.

2 participants