Merge branch 'master' into test-util-ephemeral-server-feature

.github/workflows/heavy.yml

@@ -6,6 +6,10 @@ on: # rebuild any PRs and main branch changes
     branches:
       - master
 
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
 jobs:
   build-and-test:
     timeout-minutes: 20

.github/workflows/per-pr.yml

@@ -0,0 +1,96 @@
+name: Per merge CI
+
+on: # rebuild any PRs and main branch changes
+  pull_request:
+  push:
+    branches:
+      - master
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  build-and-test:
+    name: "Format, docs, and lint"
+    timeout-minutes: 20
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+        with:
+          submodules: recursive
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: 1.74.0
+          override: true
+      - name: Install protoc
+        uses: arduino/setup-protoc@v1
+        with:
+          version: '3.x'
+          repo-token: ${{ secrets.GITHUB_TOKEN }}
+      - run: rustup component add rustfmt clippy
+      - uses: Swatinem/rust-cache@v2
+      - uses: actions-rs/cargo@v1
+        with:
+          command: fmt
+          args: --all -- --check
+      - uses: actions-rs/cargo@v1
+        with:
+          command: doc
+          args: --workspace --all-features --no-deps
+      - uses: actions-rs/cargo@v1
+        with:
+          command: lint
+      - uses: actions-rs/cargo@v1
+        with:
+          command: test-lint
+
+  test:
+    name: Unit Tests
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: 1.74.0
+          override: true
+      - name: Install protoc
+        uses: arduino/setup-protoc@v1
+        with:
+          version: '3.x'
+          repo-token: ${{ secrets.GITHUB_TOKEN }}
+      - uses: Swatinem/rust-cache@v2
+      - uses: actions-rs/cargo@v1
+        with:
+          command: test
+          args: -- --include-ignored
+      - uses: actions/upload-artifact@v3
+        with:
+          name: coverage-report
+          path: tarpaulin-report.html
+      - uses: actions/upload-artifact@v3
+        with:
+          name: state-machine-coverage
+          path: machine_coverage/
+
+  fmt:
+    name: Integ tests
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: 1.74.0
+          override: true
+      - name: Install protoc
+        uses: arduino/setup-protoc@v1
+        with:
+          version: '3.x'
+          repo-token: ${{ secrets.GITHUB_TOKEN }}
+      - uses: Swatinem/rust-cache@v2
+      - uses: actions-rs/cargo@v1
+        with:
+          command: integ-test

ARCHITECTURE.md

@@ -66,7 +66,7 @@ async fn hello_activity(name: &str) -> String {
 
 We define the interface between the core and lang SDKs in terms of gRPC service definitions. The actual implementations of this "service" are not generated by gRPC generators, but the messages themselves are, and make it easier to hit the ground running in new languages.
 
-See the latest API definition [here](https://github.com/temporalio/sdk-core/tree/master/protos/local/temporal/sdk/core)
+See the latest API definition [here](https://github.com/temporalio/sdk-core/tree/master/sdk-core-protos/protos/local/temporal/sdk/core)
 
 
 ## Other topics

README.md

@@ -48,8 +48,8 @@ You can build and test the project using cargo:
 `cargo build`
 `cargo test`
 
-Run integ tests with `cargo integ-test`. You will need to already be running the server:
-`docker-compose -f .buildkite/docker/docker-compose.yaml up`
+Run integ tests with `cargo integ-test`. By default it will start an ephemeral server. You can also
+use an already-running server by passing `-s external`.
 
 Run load tests with `cargo test --features=save_wf_inputs --test heavy_tests`.
 
@@ -79,7 +79,7 @@ The passed in options to initialization can be customized to export to an OTel c
 To run integ tests with OTel collection on, you can use `integ-with-otel.sh`. You will want to make
 sure you are running the collector via docker, which can be done like so:
 
-`docker-compose -f .buildkite/docker/docker-compose.yaml -f .buildkite/docker/docker-compose-telem.yaml up`
+`docker-compose -f docker/docker-compose.yaml -f docker/docker-compose-telem.yaml up`
 
 If you are working on a language SDK, you are expected to initialize tracing early in your `main`
 equivalent.

core-api/src/lib.rs

@@ -24,25 +24,17 @@ pub trait Worker: Send + Sync {
     /// be one outstanding activation for a particular run of a workflow at any time. If an
     /// activation is not responded to, it will cause that workflow to become stuck forever.
     ///
-    /// Activations that contain only a `remove_from_cache` job should not cause the workflow code
-    /// to be invoked and may be responded to with an empty command list. Eviction jobs may also
-    /// appear with other jobs, but will always appear last in the job list. In this case it is
-    /// expected that the workflow code will be invoked, and the response produced as normal, but
-    /// the caller should evict the run after doing so.
+    /// See [WorkflowActivation] for more details on the expected behavior of lang w.r.t activation
+    /// & job processing.
     ///
-    /// It is rarely a good idea to call poll concurrently. It handles polling the server
-    /// concurrently internally.
+    /// Do not call poll concurrently. It handles polling the server concurrently internally.
     async fn poll_workflow_activation(&self) -> Result<WorkflowActivation, PollWfError>;
 
     /// Ask the worker for some work, returning an [ActivityTask]. It is then the language SDK's
     /// responsibility to call the appropriate activity code with the provided inputs. Blocks
     /// indefinitely until such work is available or [Worker::shutdown] is called.
     ///
-    /// The returned activation is guaranteed to be for the same task queue / worker which was
-    /// provided as the `task_queue` argument.
-    ///
-    /// It is rarely a good idea to call poll concurrently. It handles polling the server
-    /// concurrently internally.
+    /// Do not call poll concurrently. It handles polling the server concurrently internally.
     async fn poll_activity_task(&self) -> Result<ActivityTask, PollActivityError>;
 
     /// Tell the worker that a workflow activation has completed. May (and should) be freely called

core/src/core_tests/updates.rs

@@ -32,11 +32,11 @@ async fn replay_with_empty_first_task() {
         task.jobs.as_slice(),
         [
             WorkflowActivationJob {
-                variant: Some(workflow_activation_job::Variant::StartWorkflow(_)),
+                variant: Some(workflow_activation_job::Variant::DoUpdate(_)),
             },
             WorkflowActivationJob {
-                variant: Some(workflow_activation_job::Variant::DoUpdate(_)),
-            }
+                variant: Some(workflow_activation_job::Variant::StartWorkflow(_)),
+            },
         ]
     );
     core.complete_workflow_activation(WorkflowActivationCompletion::from_cmd(

core/src/worker/workflow/managed_run.rs

@@ -732,7 +732,7 @@ impl ManagedRun {
         } else {
             None
         };
-        self.update_to_acts(Ok(maybe_act).map(Into::into))
+        self.update_to_acts(Ok(maybe_act.into()))
     }
     /// Returns `true` if autocompletion should be issued, which will actually cause us to end up
     /// in [completion] again, at which point we'll start a new heartbeat timeout, which will

core/src/worker/workflow/mod.rs

@@ -1392,7 +1392,7 @@ impl LocalActivityRequestSink for LAReqSink {
 /// activations must uphold.
 ///
 /// ## Ordering
-/// `patches -> signals -> other -X-> queries`
+/// `patches -> signals/updates -> other -> queries -> evictions`
 ///
 /// ## Invariants:
 /// * Queries always go in their own activation
@@ -1426,10 +1426,14 @@ fn prepare_to_ship_activation(wfa: &mut WorkflowActivation) {
             match v {
                 workflow_activation_job::Variant::NotifyHasPatch(_) => 1,
                 workflow_activation_job::Variant::SignalWorkflow(_) => 2,
+                workflow_activation_job::Variant::DoUpdate(_) => 2,
                 // In principle we should never actually need to sort these with the others, since
                 // queries always get their own activation, but, maintaining the semantic is
                 // reasonable.
                 workflow_activation_job::Variant::QueryWorkflow(_) => 4,
+                // Also shouldn't ever end up anywhere but the end by construction, but no harm in
+                // double-checking.
+                workflow_activation_job::Variant::RemoveFromCache(_) => 5,
                 _ => 3,
             }
         }
@@ -1441,14 +1445,18 @@ fn prepare_to_ship_activation(wfa: &mut WorkflowActivation) {
 mod tests {
     use super::*;
     use itertools::Itertools;
+    use temporal_sdk_core_protos::coresdk::workflow_activation::SignalWorkflow;
 
     #[test]
     fn jobs_sort() {
         let mut act = WorkflowActivation {
             jobs: vec![
                 WorkflowActivationJob {
                     variant: Some(workflow_activation_job::Variant::SignalWorkflow(
-                        Default::default(),
+                        SignalWorkflow {
+                            signal_name: "1".to_string(),
+                            ..Default::default()
+                        },
                     )),
                 },
                 WorkflowActivationJob {
@@ -1466,6 +1474,19 @@ mod tests {
                         Default::default(),
                     )),
                 },
+                WorkflowActivationJob {
+                    variant: Some(workflow_activation_job::Variant::DoUpdate(
+                        Default::default(),
+                    )),
+                },
+                WorkflowActivationJob {
+                    variant: Some(workflow_activation_job::Variant::SignalWorkflow(
+                        SignalWorkflow {
+                            signal_name: "2".to_string(),
+                            ..Default::default()
+                        },
+                    )),
+                },
             ],
             ..Default::default()
         };
@@ -1479,10 +1500,12 @@ mod tests {
             variants.as_slice(),
             &[
                 workflow_activation_job::Variant::NotifyHasPatch(_),
-                workflow_activation_job::Variant::SignalWorkflow(_),
+                workflow_activation_job::Variant::SignalWorkflow(ref s1),
+                workflow_activation_job::Variant::DoUpdate(_),
+                workflow_activation_job::Variant::SignalWorkflow(ref s2),
                 workflow_activation_job::Variant::FireTimer(_),
                 workflow_activation_job::Variant::ResolveActivity(_),
-            ]
+            ] if s1.signal_name == "1" && s2.signal_name == "2"
         )
     }
 

fsm/rustfsm_procmacro/tests/trybuild/no_handle_conversions_require_into_fail.stderr

@@ -1,8 +1,8 @@
 error[E0277]: the trait bound `One: From<Two>` is not satisfied
-  --> tests/trybuild/no_handle_conversions_require_into_fail.rs:11:18
+  --> tests/trybuild/no_handle_conversions_require_into_fail.rs:11:5
    |
 11 |     Two --(B)--> One;
-   |                  ^^^ the trait `From<Two>` is not implemented for `One`
+   |     ^^^ the trait `From<Two>` is not implemented for `One`
    |
    = note: required for `Two` to implement `Into<One>`
 note: required by a bound in `TransitionResult::<Sm, Ds>::from`