Skip to content

Conversation

@jeffschoner
Copy link
Contributor

Summary

Two problems were introduced when #224 was merged to master.

  • Errors were being raised in many test workflows about not being able to find a payload converter. The examples/spec/integration/converter_spec.rb was not properly restoring the payload code configuration to default after changing it. This caused other workflows run afterward to encrypt their payloads on workflow start, even though there was no encryption set up in the worker. This in turn resulted in those workflows entering workflow task failure loops and being unable to complete.
  • The Temporal API proto was previously on v1.16, but got accidentally got reverted back to a much earlier version from March 2021

This fixes those problems:

  • The converter spec now properly restores the payload codec field upon ensure, instead of the data converter (which is now unchanged)
  • The API proto has been moved forward to 1.20. While new features aren't needed here yet, this will be ready to go if/when new features like Schedules are added

Testing

All example specs now succeed. The spec with the mistake never failed, only corrupting state for running the others that followed.

cd examples # follow instructions in start three copies of bin/worker in the README
bundle exec rspec

Updated unit spec to check for the new, better error message:

bundle exec spec/unit/lib/temporal/connection/converter/composite_spec.rb:52

@DeRauk DeRauk merged commit 5d1c088 into coinbase:master Apr 12, 2023
@jeffschoner jeffschoner deleted the upgrade-api branch April 21, 2023 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants