Skip to content

Conversation

@dandavison
Copy link
Contributor

No description provided.

@dandavison dandavison force-pushed the bump-php branch 6 times, most recently from 6c8ae6c to 81522d7 Compare January 16, 2025 22:45
@dandavison dandavison changed the title Bump PHP version Fix CI Jan 16, 2025
public function run()
{
yield Workflow::await(fn(): bool => $this->counter >= 2);
yield Workflow::await(fn(): bool => Workflow::allHandlersFinished());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@dandavison dandavison Jan 16, 2025

Choose a reason for hiding this comment

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

This is addressing the new server change to update errors: when a workflow completes and an update is in Accepted state, that used to be gRPC NOT FOUND, but is now an update failure. All SDKs are having to address this change today, because temporal CLI recently was updated to the new server version. Without the change here, we are getting

AcceptedUpdateCompletedWorkflow: Workflow Update failed because the Workflow completed before the Update completed.

I have to admit I don't fully understand where the gRPC NOT FOUND was being handled, but the fix here seems like good practice and fixes the test.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think this change is fine for the purposes of this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can just suppress this exception: temporalio/sdk-php@0f0ceec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @roxblnfk. So I guess both work -- I think I'm finding the wait allHandlersFinished a bit cleaner since it's something that we recommend. WDYT?

Copy link
Collaborator

@roxblnfk roxblnfk Jan 17, 2025

Choose a reason for hiding this comment

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

What about $this->counter >= 2 && Workflow::allHandlersFinished()?
Looks like that counter may be important

Yeah. Looks good for me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@dandavison
There should be a condition $this->counter >= 2 && Workflow::allHandlersFinished() because the Workflow might start and finish before the Update methods are called.

@dandavison
Copy link
Contributor Author

There are two remaining problems here:

  docker run --rm -i -v /tmp/temporal-certs:/certs features:php-2.12.3 \
  --server $TEMPORAL_CLOUD_ADDRESS \
  --namespace $TEMPORAL_CLOUD_NAMESPACE \
  --client-cert-path /certs/client.pem \
  --client-key-path /certs/client.key
  shell: /usr/bin/bash -e {0}
  env:
    TEMPORAL_CLOUD_ADDRESS: sdk-ci.a2dd6.tmprl.cloud:7233
    TEMPORAL_CLOUD_NAMESPACE: sdk-ci.a2dd6
    REPO_URL: https://github.com/temporalio/features
    FEATURES_BUILT_IMAGE_TAG: features:php-2.12.3
    FEATURES_BUILT_IMAGE_TAGS: features:php-2.12.3;features:php-2.12;features:php-2;features:php
Starting RoadRunner... error
Error starting RoadRunner: handle_serve_command: Function call error:
	serve error from the plugin *rrtemporal.Plugin stopping execution, error: temporal_plugin_serve: failed reaching server: last connection error: connection error: desc = "error reading server preface: read tcp 172.17.0.2:53200->44.241.31.151:7233: read: connection reset by peer"

and

  docker run --rm -i -v /tmp/temporal-certs:/certs features:go-1.32.1 \
  --server $TEMPORAL_CLOUD_ADDRESS \
  --namespace $TEMPORAL_CLOUD_NAMESPACE \
  --client-cert-path /certs/client.pem \
  --client-key-path /certs/client.key

panic: 
	Error Trace:	/app/features/update/updateutil/history.go:20
	            				/app/features/update/activities/feature.go:49
	            				/app/harness/go/harness/runner.go:120
	            				/app/harness/go/cmd/run.go:271
	            				/app/harness/go/cmd/run.go:230
	            				/app/harness/go/cmd/run.go:246
	            				/app/harness/go/cmd/run.go:38
	            				/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/command.go:163
	            				/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/app.go:313
	            				/go/pkg/mod/github.com/urfave/cli/v2@v2.3.0/app.go:224
	            				/app/harness/go/cmd/cmd.go:20
	            				/app/prepared/main.go:9
	            				/usr/local/go/src/runtime/proc.go:267
	            				/usr/local/go/src/runtime/asm_amd64.s:1650
	Error:      	Received unexpected error:
	            	after 5s, no workflow(s) found

@rustatian
Copy link

Hey @dandavison 👋🏻
RR can't connect to the temporal, looks like either address in the .rr.yaml is not correct or RR (by the provided address) don't have an access to the temporal. RR uses SDK-GO under the hood.

@rustatian
Copy link

I double checked the configuration (.rr.yaml) and the problem is in TLS I suppose. RR uses the regular, non-encrypted connection, but you're trying to connect using TLS.
You need to update RR's configuration to include these certs (or envs pointing to these certs) as shown here: link

@dandavison
Copy link
Contributor Author

Thanks @rustatian! Do you know what recent change might have caused this to break?

@rustatian
Copy link

Hard to say, might be some update to the Temporal cloud (or instance located by this address 44.241.31.151:7233) which is now requires a TLS connection only... As far as I know, this is not a SDK-GO update.

@roxblnfk
Copy link
Collaborator

roxblnfk commented Jan 17, 2025

You can uncomment this line to check all the command line parameters

// echo "\e[1;36mStart RoadRunner with command:\e[0m {$command}\n";


The reason might be that the temporal. prefix is missing there:

$run->tlsKey === null or $rrCommand = [...$rrCommand, '-o', "tls.key={$run->tlsKey}"];
$run->tlsCert === null or $rrCommand = [...$rrCommand, '-o', "tls.cert={$run->tlsCert}"];

temporal.tls.key, temporal.tls.cert

@dandavison dandavison force-pushed the bump-php branch 3 times, most recently from 1d61409 to 4cab04c Compare January 17, 2025 19:01
@dandavison dandavison merged commit baf991d into main Jan 17, 2025
21 checks passed
@dandavison dandavison deleted the bump-php branch January 17, 2025 20:07
mihaelabalas84 added a commit to fairmoney/temporal-features that referenced this pull request Dec 16, 2025
* Build docker images on all main CI jobs (temporalio#545)

* Add PHP dockerfile building (temporalio#541)

Co-authored-by: Josh Berry <josh.berry@temporal.io>

* Publish PHP in the Dockerhub (temporalio#546)

* Centralize dynamic config (temporalio#543)

* Update update tests for Java SDK v1.26.1 (temporalio#555)

Update SDK to support Java SDK v1.26.0

* Update gradle to v8 (temporalio#554)

* PHP SDK integration (temporalio#485)

* feat: add PHP initialization

* chore(PHP): implement RoadRunner run command generator

* feat(PHP): add ClassLocator; implement Workflow and Activity classes detection and loading

* chore(PHP): RoadRunner now is run from PHP script that will start also client code

* chore(PHP): implemented checks starting; refactoring; implemented query/successful_query feature

* chore(PHP): better notification about failures

* fix(PHP): fixed task queue binding and workflow run with correct task queue

* chore(PHP): add workflow injector that runs workflow with correct queue and options based on an attribute

* feat(PHP): add the RoadRunner runner service to be able to stop and rerun RoadRunner; add `query/timeout_due_to_no_active_workers` feature

* chore(PHP): add feature `query/unexpected_arguments`

* chore(PHP): finish all the `query` features

* chore(PHP): add activity features: `basic_no_workflow_timeout`, `cancel_try_cancel` and `retry_on_error`

* chore(PHP): add Child Workflow feature `signal`

* chore(PHP): add Child Workflow features: `result` and `throws_on_execute`

* chore(PHP): add feature `continue_as_new/continue_as_same`

* chore(PHP): add feature `eager_workflow/successful_start"`

* feat(PHP): add ability to inject client Interceptor provider via feature attributes

* chore(PHP): add feature `data_converter/json`

* chore(PHP): add feature `data_converter/json_protobuf`

* chore(PHP): add feature `data_converter/empty`

* chore(PHP): add feature `data_converter/failure`

* feat(PHP): support custom Payload Converters in client and server sides

* chore(PHP): add feature `data_converter/codec`

* fix(PHP): fix constants conflict in feature files; place binary proto converter after the json proto converter

* chore(PHP): add feature `data_converter/binary_protobuf`

* chore(PHP): add feature `data_converter/binary`

* chore(PHP): add feature `eager_activity/non_remote_activities_worker`

* chore(PHP): add feature `schedule\backfill`

* chore(PHP): add feature `schedule\basic`

* chore(PHP): add feature `schedule\pause`

* chore(PHP): add feature `schedule\trigger`

* chore(PHP): add feature `signal\activities`

* chore(PHP): add feature `signal\basic`

* chore(PHP): add feature `signal\child_workflow`

* chore(PHP): add feature `signal\external`

* chore(PHP): add feature `signal\prevent_close`

* chore(PHP): add feature `signal\signal_with_start`

* chore(PHP): add case into the `signal\signal_with_start` feature

* chore(PHP): add case `update\activities`

* chore(PHP): add case `update\async_accepted`

* chore(PHP): add case `update/basic`

* chore(PHP): add case `update/basic_async`

* chore(PHP): add case `update/client_interceptor`

* chore(PHP): add case `update/deduplication`

* chore(PHP): add case `update/non_durable_reject`

* chore(PHP): add case `update/self`

* chore(PHP): add case `update/task_failure`

* chore(PHP): add case `update/validation_replay`

* feat(PHP): configure KV module; add case `update/worker_restart`

* chore(PHP): support testing in a separated dir

* chore(PHP): add PHP dockerfile; cleanup

* chore(PHP): fix PHP dockerfile; add github workflow

* chore(PHP): polish update/* tests

* chore(PHP): fix todos

* chore: Sync with PHP SDK 2.11

* ci: Fix PHP version detection before Docker image building

* ci: add commands to build PHP image

* ci: add prefix `v` for php-ver input

* ci: fix typo

* Ignore platform req on composer install

* Skip data_converter/failure last check

* Skip one of update/task_failure tests

* Fix schedule/basic test: add 10 sec timeout to find schedule

* Fix installing dependencies on PHP image building

* Optimize PHP dockerfile

* Mark eager_activity tests skipped

* Add GITHUB_TOKEN to download RoadRunner without a limit

* Improve comment about BuildPhpProgramOptions.Version

* Skip `eager_workflow` test if the server doesn't support it on the ServerCapabilities level

* Fix dynamic config values being passed to cli

* Replace `frontend` with `system` for `forceSearchAttributesCacheRefreshOnRead`,  `enableActivityEagerExecution` and `enableEagerWorkflowStart` options

* Include `dynamicconfig` into php docker image

---------

Co-authored-by: Spencer Judge <spencer@temporal.io>

* PHP: fix a floating error in the worker_restart feature check (temporalio#570)

* Upgrade go sdk, ignore Deployment history field (temporalio#578)

* Fix CI (temporalio#577)

* Bump PHP version

* Log at WARN level

* Fix PHP test

* Bump sdk-go

* Scrub new Deployment field

* Use temporal. prefix

* Skip check with env var

* Skip failing PHP test

* Delete PHP feature

* Python: reduce log spam and show failed features (temporalio#574)

* Reduce log spam and show failed features

* Replace ListOpenWorkflow and ListClosedWorkflow with ListWorkflow (temporalio#579)

* Skip one more history check in cloud (temporalio#583)

* Skip one more history check in cloud

* Bump python 1.7.0 => 1.9.0 (temporalio#569)

* Use EventuallyWithT to prevent reset_and_delete feature flakiness (temporalio#586)


---------

Co-authored-by: Chad Retz <chad.retz@gmail.com>

* updates_do_not_block_continue_as_new (temporalio#584)

* Test update doesn't block CAN and is handled on next run

* Rename Go harness Runner.Assert to SoftAssert and document it (temporalio#587)

* uv + maturin migration (temporalio#600)

* [PHP] Update CI and tests (temporalio#601)

* Restore PHP workflows

* Use actions/download-artifact v4

* Enable logging for RR run command

* Fix workflow id in `continue as new` test

* Add debugging on date interval unmarshalling

* Start workflows with WorkflowIdReusePolicy as `AllowDuplicate`
Remove debug

* Update min PHP SDK version

* Increase RPC operations timeouts

* Add deployment version features (temporalio#611)

* Don't rely on TS FailureConverter returning a TemporalFailure (temporalio#618)

See related change: temporalio/sdk-typescript#1685

Also upgraded Go in docker image to ensure the Go image build action passes.

* Bump sdk version (temporalio#621)

* bump sdk-python (temporalio#622)

* bump sdk-python

* Add CODEOWNERS

* Pin protobufjs to 7.5.1 (temporalio#625)

* Fixed SDK version mismatch in .Net build. Made `--version` optional in `prepare`. Updated vulnerable dependencies for .Net. (temporalio#628)

Previously, .Net harness and tests were built against the hardcoded SDK version but run with the version specified in command line. This would make the code sometimes fail mysteriously due to binary incompatibilities. Now, the harness and tests are always built and run with the same SDK version.

`run` command already supported omitting `--version` argument (individual languages still do a check if it's required for them). `prepare` still required it for no real reason, so I changed it to match `run`'s behavior.

.Net SDK 9 changed behavior of NuGet audit to also check transitive dependencies by default. We were using a few vulnerable transitive dependencies, which caused .Net build to fail when using .Net 9.0 (it works with 8.0). Updating dependencies fixed the issue.

* Bump python (temporalio#629)

* TypeScript: Remove pin on grpc-js v1.10.10 (temporalio#626)

* Bump Python SDK (temporalio#638)

* Explicitly use rust in python build (temporalio#640)

* Fix flake in timeout_due_to_no_active_workers on TypeScript (temporalio#641)

* Update Rust to at least 1.88 (temporalio#642)

* Add missing TS dependency to @grpc/grpc-js (temporalio#643)

* Versioning breaking changes (temporalio#631)

* Update gomod to use unreleased version

* Handle breaking changes

* Update gomods

---------

Co-authored-by: Andrew Yuan <theandrewyuan@gmail.com>

* Fix broken pyproj version lookup (temporalio#654)

* Update graceful worker shutdown (temporalio#627)

* Edit README for shutdown

* PR feedback

* heartbeating blurb

* Be more specific about core vs. lang timeout behavior

* Add links, add TS TODO

* Build with custom stdout/stderr (temporalio#662)

* [TS] Fix the client injection activity interceptor conflicting with new upstream API (temporalio#667)

* Activity shutdown tests (temporalio#660)

* Wrote Go test

* Added Java

* Dotnet done

* ts and python

* clean up comments

* formatting

* fix TS test

* clean up, increase timeout to allow for 3rd activity to be scheduled

* test seems fine locally, run in CI with extra prints

* test passed in CI, remove prints

* wait on event instead of sleep

* forgot to run format on python and java

* Offline Python (temporalio#679)

* Add TLS Server Name Config (temporalio#686)

* Update TS Dockerfile to Node 22/Bullseye (temporalio#688)

* Bump Python SDK version and saner version syntax (temporalio#668)

* Handle if crt/key are Uint8Array (temporalio#689)

* Handle if crt/key are Uint8Array

* formatting/linting

* [PHP] Add test case for child_workflow/cancel_abandon (temporalio#644)

* Add test case for child_workflow/cancel_abandon

* Resolve TODO

* Update Cancel Abandoned Child Workflow tests

* Add close method to cancel workflow

* Add the ability to pass the ca cert for doing mtls server verification (temporalio#690)

* Set explicit permissions for GitHub Actions workflows (temporalio#693)

* Update to python >= 3.10 and sdk-python 1.19.0 (temporalio#699)

* Update to python >= 3.10 and sdk-python 1.19.0

* update features python program to new min python version

* chore(typescript): switch to pnpm (temporalio#698)

* chore: switch to pnpm

* skip setting up pnpm cache

* update python sdk to 1.20.0 (temporalio#704)

* remove autosetup (temporalio#707)

* remove autosetup

* casing and remove env vars that are not needed

* Revert "remove autosetup (temporalio#707)" (temporalio#710)

This reverts commit c1fa10c.

* Remove Autosetup and Add default namespace creation for GHA (temporalio#709)

* create default namespace after server start

* add some debugging logs

* more logging

* move to local docker compose

* add retries to test with cloud

* log heatlh check response

* refine the restart config and add some logging to the curl command

* try different retry logic

* Use FM forked repos

---------

Co-authored-by: Roey Berman <roey@temporal.io>
Co-authored-by: Aleksei Gagarin <roxblnfk@ya.ru>
Co-authored-by: Josh Berry <josh.berry@temporal.io>
Co-authored-by: Spencer Judge <spencer@temporal.io>
Co-authored-by: Quinn Klassen <klassenq@gmail.com>
Co-authored-by: Ivan Gantsev <ivangancev@yandex.ru>
Co-authored-by: Antonio Lain <135073478+antlai-temporal@users.noreply.github.com>
Co-authored-by: Dan Davison <dan.davison@temporal.io>
Co-authored-by: Chad Retz <chad.retz@gmail.com>
Co-authored-by: James Watkins-Harvey <mjameswh@users.noreply.github.com>
Co-authored-by: Maciej Dudkowski <maciej.dudkowski@temporal.io>
Co-authored-by: tconley1428 <tconley1428@gmail.com>
Co-authored-by: Andrew Yuan <theandrewyuan@gmail.com>
Co-authored-by: Andrew Yuan <andrew.yuan@temporal.io>
Co-authored-by: Stephan Behnke <stephanos@users.noreply.github.com>
Co-authored-by: kepe-temporal <kepe.bonner@temporal.io>
Co-authored-by: Thomas Hardy <thestaffofmoses@gmail.com>
Co-authored-by: Kent Gruber <kent.picat.gruber@gmail.com>
Co-authored-by: Alex Mazzeo <alex.mazzeo@temporal.io>
Co-authored-by: Chris Olszewski <chrisdolszewski@gmail.com>
Co-authored-by: Alex Stanfield <13949480+chaptersix@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants