Rest of Nexus API definitions #363

bergundy · 2024-03-06T23:24:24Z

Note for reviewers

Server support is implemented in the nexus feature branch.
We should wait until we're ready to merge nexus into main.

Even after we merge this, there'll like be very small changes as we work on the Go SDK, but hopefully only additions, I tried my best not to require any more breaking changes.

bergundy · 2024-03-07T01:01:37Z

Changed to draft, I already see some gaps.

cretz · 2024-03-07T13:10:46Z

temporal/api/command/v1/message.proto

@@ -234,6 +234,27 @@ message ProtocolMessageCommandAttributes {
    string message_id = 1;
 }

+message ScheduleNexusOperationCommandAttributes {


Can I set headers like I can via Nexus calls directly?

Not yet, but I suspect we'll need this later for tracing. Might as well just add it now.

cretz · 2024-03-07T13:17:48Z

temporal/api/enums/v1/command_type.proto

@@ -49,4 +49,6 @@ enum CommandType {
    COMMAND_TYPE_UPSERT_WORKFLOW_SEARCH_ATTRIBUTES = 13;
    COMMAND_TYPE_PROTOCOL_MESSAGE = 14;
    COMMAND_TYPE_MODIFY_WORKFLOW_PROPERTIES = 16;
+    COMMAND_TYPE_SCHEDULE_NEXUS_OPERATION = 17;
+    COMMAND_TYPE_CANCEL_NEXUS_OPERATION = 18;


Suggested change

COMMAND_TYPE_CANCEL_NEXUS_OPERATION = 18;

COMMAND_TYPE_REQUEST_CANCEL_NEXUS_OPERATION = 18;

cretz · 2024-03-07T13:18:37Z

temporal/api/enums/v1/common.proto

+    // Operation completed as canceled (may have not ever been delivered).
+    NEXUS_OPERATION_STATE_CANCELED = 7;
+    // Operation timed out.
+    NEXUS_OPERATION_STATE_TIMEDOUT = 8;


Suggested change

NEXUS_OPERATION_STATE_TIMEDOUT = 8;

NEXUS_OPERATION_STATE_TIMED_OUT = 8;

Makes sense, thanks.

temporal/api/enums/v1/common.proto

cretz · 2024-03-07T13:19:55Z

temporal/api/enums/v1/common.proto

+    // Operation has just been initialized, waiting to be scheduled.
+    // This is a transient state which may never be visible to a client.
+    NEXUS_OPERATION_STATE_INITIALIZED = 1;


To confirm though, this is a state it may be in? I would think that any Nexus operation goes straight to "scheduled".

Today it's not a state an operation will ever be in because it immediately transitions to the scheduled state.
In the future we may block operations from being scheduled if the queue per source namespace and destination service is backed up.

I was debating whether this should added here or not, still not sure if I want to initialize the state machine on the server in the UNSPECIFIED state or have this explicit state before transitioning to scheduled.

I'll have to get deeper into the implementation to decide.

cretz · 2024-03-07T13:21:21Z

temporal/api/workflowservice/v1/request_response.proto

@@ -861,6 +861,7 @@ message DescribeWorkflowExecutionResponse {
    repeated temporal.api.workflow.v1.PendingChildExecutionInfo pending_children = 4;
    temporal.api.workflow.v1.PendingWorkflowTaskInfo pending_workflow_task = 5;
    repeated temporal.api.workflow.v1.CallbackInfo callbacks = 6;
+    repeated temporal.api.workflow.v1.NexusOperationInfo pending_nexus_operations = 7;


I note this doesn't match the other "pending" whose info objects are specific to pending and don't include state enumerates for completion. Are you sure that you shouldn't call the object PendingNexusOperationInfo and the enum PendingNexusOperationState and get rid of the terminal states? If you think you may want a more robust info, I think you should go ahead and map out the RPC that uses it. Otherwise, you have multiple unused enum values from an API POV.

Yeah, maybe these states should be internally defined in the server repo.
I'll probably want to delete the operation state machines from mutable state after completion.

👍 To me from an API POV a pending-specific info with a pending-specific state enum matches what we have and doesn't cause any problems with attempted inadvertent reuse of this info for anything else. (for sure actual server model may differ)

I'll remove the non-pending states.

cretz · 2024-03-07T13:23:21Z

temporal/api/enums/v1/event_type.proto

+    // A Nexus operation completed as canceled.
+    EVENT_TYPE_NEXUS_OPERATION_CANCELED = 52;
+    // A Nexus operation timed out.
+    EVENT_TYPE_NEXUS_OPERATION_TIMEDOUT = 53;


Suggested change

EVENT_TYPE_NEXUS_OPERATION_TIMEDOUT = 53;

EVENT_TYPE_NEXUS_OPERATION_TIMED_OUT = 53;

cretz · 2024-03-07T13:24:03Z

temporal/api/history/v1/message.proto

+        NexusOperationStartedEventAttributes nexus_operation_started_event_attributes = 54;
+        NexusOperationCompletedEventAttributes nexus_operation_completed_event_attributes = 55;
+        NexusOperationFailedEventAttributes nexus_operation_failed_event_attributes = 56;
+        NexusOperationCanceledEventAttributes nexus_operation_canceld_event_attributes = 57;


Suggested change

NexusOperationCanceledEventAttributes nexus_operation_canceld_event_attributes = 57;

NexusOperationCanceledEventAttributes nexus_operation_canceled_event_attributes = 57;

cretz · 2024-03-07T13:24:12Z

temporal/api/history/v1/message.proto

+        NexusOperationCompletedEventAttributes nexus_operation_completed_event_attributes = 55;
+        NexusOperationFailedEventAttributes nexus_operation_failed_event_attributes = 56;
+        NexusOperationCanceledEventAttributes nexus_operation_canceld_event_attributes = 57;
+        NexusOperationTimedOutEventAttributes nexus_operation_timedout_event_attributes = 58;


Suggested change

NexusOperationTimedOutEventAttributes nexus_operation_timedout_event_attributes = 58;

NexusOperationTimedOutEventAttributes nexus_operation_timed_out_event_attributes = 58;

cretz · 2024-03-07T13:28:50Z

temporal/api/workflow/v1/message.proto

@@ -183,3 +183,28 @@ message CallbackInfo {
    // The time when the next attempt is scheduled.
    google.protobuf.Timestamp next_attempt_schedule_time = 8;
 }
+
+message NexusOperationInfo {


I am wondering if we should have a concept of worker identity reported upon poll with Nexus task handlers. It is useful information for activities and such, so it's probably useful here too.

This is information for the caller. The handler may not even be a worker and I'm not sure the handler side will want to expose worker identities.

I think I'll probably add the operation ID at minimum here.

I see, I agree due to the needed separation, we should not report this worker information to the caller. However it does bring up the question, as a caller I can see outstanding Nexus operations, but can I on the incoming/impl side? Maybe it's not a big deal for now since it's usually 1:1 with workflows.

Not a big deal now, it's 1:1 with workflows and you'll be able to see that there are nexus callbacks attached to your workflow.

Later on we'll want better tracing abilities but I haven't gotten to that yet.

bergundy

Thanks @cretz for the great feedback.

bergundy · 2024-03-07T17:27:34Z

temporal/api/command/v1/message.proto

@@ -234,6 +234,27 @@ message ProtocolMessageCommandAttributes {
    string message_id = 1;
 }

+message ScheduleNexusOperationCommandAttributes {


Not yet, but I suspect we'll need this later for tracing. Might as well just add it now.

bergundy · 2024-03-07T17:27:54Z

temporal/api/enums/v1/command_type.proto

@@ -49,4 +49,6 @@ enum CommandType {
    COMMAND_TYPE_UPSERT_WORKFLOW_SEARCH_ATTRIBUTES = 13;
    COMMAND_TYPE_PROTOCOL_MESSAGE = 14;
    COMMAND_TYPE_MODIFY_WORKFLOW_PROPERTIES = 16;
+    COMMAND_TYPE_SCHEDULE_NEXUS_OPERATION = 17;
+    COMMAND_TYPE_CANCEL_NEXUS_OPERATION = 18;


bergundy · 2024-03-07T17:31:21Z

temporal/api/enums/v1/common.proto

+    // Operation has just been initialized, waiting to be scheduled.
+    // This is a transient state which may never be visible to a client.
+    NEXUS_OPERATION_STATE_INITIALIZED = 1;


Today it's not a state an operation will ever be in because it immediately transitions to the scheduled state.
In the future we may block operations from being scheduled if the queue per source namespace and destination service is backed up.

I was debating whether this should added here or not, still not sure if I want to initialize the state machine on the server in the UNSPECIFIED state or have this explicit state before transitioning to scheduled.

I'll have to get deeper into the implementation to decide.

temporal/api/enums/v1/common.proto

bergundy · 2024-03-07T17:33:28Z

temporal/api/enums/v1/common.proto

+    // Operation completed as canceled (may have not ever been delivered).
+    NEXUS_OPERATION_STATE_CANCELED = 7;
+    // Operation timed out.
+    NEXUS_OPERATION_STATE_TIMEDOUT = 8;


Makes sense, thanks.

bergundy · 2024-03-07T17:33:32Z

temporal/api/enums/v1/event_type.proto

+    // A Nexus operation completed as canceled.
+    EVENT_TYPE_NEXUS_OPERATION_CANCELED = 52;
+    // A Nexus operation timed out.
+    EVENT_TYPE_NEXUS_OPERATION_TIMEDOUT = 53;


bergundy · 2024-03-07T17:33:41Z

temporal/api/history/v1/message.proto

+        NexusOperationStartedEventAttributes nexus_operation_started_event_attributes = 54;
+        NexusOperationCompletedEventAttributes nexus_operation_completed_event_attributes = 55;
+        NexusOperationFailedEventAttributes nexus_operation_failed_event_attributes = 56;
+        NexusOperationCanceledEventAttributes nexus_operation_canceld_event_attributes = 57;


bergundy · 2024-03-07T17:33:48Z

temporal/api/history/v1/message.proto

+        NexusOperationCompletedEventAttributes nexus_operation_completed_event_attributes = 55;
+        NexusOperationFailedEventAttributes nexus_operation_failed_event_attributes = 56;
+        NexusOperationCanceledEventAttributes nexus_operation_canceld_event_attributes = 57;
+        NexusOperationTimedOutEventAttributes nexus_operation_timedout_event_attributes = 58;


bergundy · 2024-03-07T17:35:06Z

temporal/api/workflow/v1/message.proto

@@ -183,3 +183,28 @@ message CallbackInfo {
    // The time when the next attempt is scheduled.
    google.protobuf.Timestamp next_attempt_schedule_time = 8;
 }
+
+message NexusOperationInfo {


This is information for the caller. The handler may not even be a worker and I'm not sure the handler side will want to expose worker identities.

I think I'll probably add the operation ID at minimum here.

bergundy · 2024-03-07T17:36:33Z

temporal/api/workflowservice/v1/request_response.proto

@@ -861,6 +861,7 @@ message DescribeWorkflowExecutionResponse {
    repeated temporal.api.workflow.v1.PendingChildExecutionInfo pending_children = 4;
    temporal.api.workflow.v1.PendingWorkflowTaskInfo pending_workflow_task = 5;
    repeated temporal.api.workflow.v1.CallbackInfo callbacks = 6;
+    repeated temporal.api.workflow.v1.NexusOperationInfo pending_nexus_operations = 7;


Yeah, maybe these states should be internally defined in the server repo.
I'll probably want to delete the operation state machines from mutable state after completion.

cretz

This looks good to me (not marking a draft "approved" though)

bergundy · 2024-04-19T17:55:28Z

temporal/api/nexus/v1/message.proto

@@ -39,7 +38,7 @@ import "temporal/api/common/v1/message.proto";
 message Failure {
    string message = 1;
    map<string, string> metadata = 2;
-    google.protobuf.Value details = 3;
+    bytes details = 3;


Note this is a backwards incompatible change but I realized it's simpler to just use JSON serialized to bytes here.

How is this set by Nexus handlers and the Nexus HTTP protocol?

In Go there's a nexus.Failure object with a details field that's json.RawMessage.

https://github.com/nexus-rpc/sdk-go/blob/506896b352dacee3b5d7d62010a87bc08b2b8fd1/nexus/api.go#L48-L55

Here's how it's defined in the HTTP spec: https://github.com/nexus-rpc/api/blob/main/SPEC.md#failure

👍 I agree that bytes is better than structured value here. May want to document that it is likely JSON based on SDK usage.

temporal/api/command/v1/message.proto

rodrigozhou

LGTM, please address @pdoerner comments.

cretz

How far along is the implementation? Is it possible to not put these history-affecting protos into master until the implementation is basically complete server side?

cretz · 2024-04-22T13:05:53Z

temporal/api/enums/v1/common.proto

+}
+
+// State of a Nexus operation cancelation.
+enum NexusOperationCancelationState {


While we are a bit inconsistent on "cancelled" vs "canceled", we are consistently using Cancellation with two Ls in our protos

I knew we were inconsistent but never noticed we we writing Cancellation with two Ls.
I'm totally okay changing for "consistency".

👍 Yeah we're consistent on cancellation, it's canceled we're inconsistent on

cretz · 2024-04-22T13:08:38Z

temporal/api/workflow/v1/message.proto

+    // The time when the next attempt is scheduled.
+    google.protobuf.Timestamp next_attempt_schedule_time = 10;
+
+    NexusOperationCancelationInfo cancelation_info = 11;


We don't seem to report this information for activities today, are we sure we have to put it on Nexus operations at this time?

Same as above.

cretz · 2024-04-22T13:09:17Z

temporal/api/workflow/v1/message.proto

+    // The time when the last attempt completed.
+    google.protobuf.Timestamp last_attempt_complete_time = 8;
+    // The last attempt's failure, if any.
+    temporal.api.failure.v1.Failure last_attempt_failure = 9;
+    // The time when the next attempt is scheduled.
+    google.protobuf.Timestamp next_attempt_schedule_time = 10;


We don't seem to report these 3 values for activities today, are we sure we have to put it on Nexus operations at this time?

I think this is important to be able to understand how your operation is progressing.
Activities are run by your workers, operations requests are made by the server, there's no other means of inspecting the status.

Arguably it's important for activities too and I can give obvious reasons why, but we don't wait on whether I think it's important, we wait on whether users do so we don't prematurely make the API surface too large. Ideally Nexus operations and activities can have similar visibility if/until more is needed in general. We are working too hard to make Nexus special in some of these cases based on these assumptions of need.

Activity has last started and last failure.
Operations don't record attempt starts, only completions, so it's basically exposing the same amount of information but in different format.

cretz · 2024-04-22T13:15:10Z

temporal/api/command/v1/message.proto

+    // This is useful propagating tracing information.
+    // Note that it is not using temporal.api.common.v1.Header because the Nexus protocol only supports string header
+    // values.
+    map<string, string> header = 5;


Note that it is not using temporal.api.common.v1.Header because the Nexus protocol only supports string header values

We could only accept string payloads, but payloads are important because they imply encrypt-ability and plugin to every SDKs encryption tooling. Doing this as just strings means callers have to use separate encryption tooling which is a lot to ask inside of a workflow.

I wonder if we can say: "json/plain" strings are unwrapped and used as strings, all other "json/*" values are just bytes converted to strings, and all other values are just base64'd bytes.

While it may be worth thinking the SDK side can just accept strings and go through the data converter which will encrypt them, but then turn them back to strings here, several users have proxies that walk protos and encrypt all payloads.

I wonder if we can say: "json/plain" strings are unwrapped and used as strings, all other "json/*" values are just bytes converted to strings, and all other values are just base64'd bytes.

I'm open to this idea but it would require additional metadata to decode and would complicate the integration between Temporal Nexus and "plain" Nexus.

several users have proxies that walk protos and encrypt all payloads.

I'm aware, I didn't expect users to put sensitive information in here.
AFAIR, our SDK headers don't go through codecs today and it doesn't seem like it's been an issue yet although I anticipate it may become an issue if they propagate baggage with their traces.

We have a concept of Temporal headers that people expect to be able to put context values including sensitive ones. To make Nexus's use of Temporal headers the one special one that has different rules is a bit off. I understand it complicates integration, but it is important to think about the user here.

How would my Go context propagator work for Nexus operations for instance? (which people absolutely put sensitive information in)

I'm not convinced it should just work by default, it all depends on how much context you want to share with the party you're calling, which may or may not be the same org and using the same encryption keys.

Header propagation does not work by default, users must write it. No keys are auto-forwarded.

After off-PR discussion, Temporal headers are not going to be Nexus headers at this time unfortunately. But we should consider that in the future.

cretz · 2024-04-22T13:19:46Z

temporal/api/nexus/v1/message.proto

@@ -39,7 +38,7 @@ import "temporal/api/common/v1/message.proto";
 message Failure {
    string message = 1;
    map<string, string> metadata = 2;
-    google.protobuf.Value details = 3;
+    bytes details = 3;


How is this set by Nexus handlers and the Nexus HTTP protocol?

cretz · 2024-04-22T13:20:24Z

temporal/api/nexus/v1/message.proto

@@ -184,4 +183,10 @@ message OutgoingService {
 message OutgoingServiceSpec {
    // URL to invoke requests.
    string url = 1;
+    // Callback URL that is attached to every StartOperation request for this service.
+    // The URL should be accessable to the handler of the StartOperation requests.


Suggested change

// The URL should be accessable to the handler of the StartOperation requests.

// The URL should be accessible to the handler of the StartOperation requests.

cretz · 2024-04-22T13:21:19Z

temporal/api/nexus/v1/message.proto

@@ -184,4 +183,10 @@ message OutgoingService {
 message OutgoingServiceSpec {
    // URL to invoke requests.
    string url = 1;
+    // Callback URL that is attached to every StartOperation request for this service.
+    // The URL should be accessable to the handler of the StartOperation requests.
+    // The service handles callbacks in the /api/v1/namespaces/{namespace_name}/nexus/callback route. Note that if the


There is discussion of removing the /api/v1 prefix from our HTTP API annotations (it is a bit redundant/unnecessary). This decision should happen soon, don't bake too many Nexus expectations on this prefix yet.

Sure, I can update the comment when the decision is made but I think it's valuable to help users set this field properly.

👍 Just a heads up so there are no implementation-side expectations inside server dev

bergundy · 2024-04-22T16:03:35Z

How far along is the implementation? Is it possible to not put these history-affecting protos into master until the implementation is basically complete server side?

Implementation is pretty much done but not yet production ready.
I have a feature branch in the server that I want to merge pending this PR.

cretz

LGTM. I do think it's unfortunate that Temporal headers can't be used.

temporal/api/command/v1/message.proto

temporal/api/history/v1/message.proto

bergundy · 2024-04-23T14:19:15Z

LGTM. I do think it's unfortunate that Temporal headers can't be used.

We can add that later, we left room/time for that.

## What changed? [Operation state machine](#5545) [Nexus command processing](#5546) [Implement Nexus start operation task executors](#5686) [Implement Nexus async completion endpoint](#5735) Upgrade API to support full E2E flow temporalio/api#363

bergundy requested review from a team as code owners March 6, 2024 23:24

bergundy marked this pull request as draft March 7, 2024 01:01

cretz reviewed Mar 7, 2024

View reviewed changes

bergundy commented Mar 7, 2024

View reviewed changes

cretz reviewed Mar 8, 2024

View reviewed changes

bergundy force-pushed the nexus-e2e branch from 5f3b855 to b3f5b3a Compare March 11, 2024 18:13

bergundy force-pushed the nexus-e2e branch 2 times, most recently from 23af171 to 3a4b81f Compare March 19, 2024 21:13

bergundy force-pushed the nexus-e2e branch 2 times, most recently from d6079af to 6f920a7 Compare April 2, 2024 17:10

bergundy force-pushed the nexus-e2e branch 4 times, most recently from a2b6afa to 73108f7 Compare April 10, 2024 17:25

bergundy added 14 commits April 15, 2024 13:26

Nexus commands and events

fd7d472

More operation states

891e666

More operation info

0af1a45

Address review comments

eb679ac

Add operation ID to PendingNexusOperationInfo

346b628

Remodel cancelation

df74956

Add failed causes

fd057dc

Minor fixes

07f88a4

Add WORKFLOW_TASK_FAILED_CAUSE_FEATURE_DISABLED

3b9609f

Change NexusOperationCanceledEventAttributes details to failure

be05da2

Failure and more OutgoingServiceSpec

4a4f505

Encryption keys, contd

e077d1f

Remove started_event_id

8e73833

Minimal nexus outgoing config

c9b17bc

bergundy added 2 commits April 15, 2024 13:26

Small changes

de8a81e

Nexus failure details as bytes + small comment

389d9a8

bergundy force-pushed the nexus-e2e branch from 73108f7 to 389d9a8 Compare April 15, 2024 20:30

bergundy commented Apr 19, 2024

View reviewed changes

Cleanup

d14f50a

bergundy marked this pull request as ready for review April 19, 2024 17:57

pdoerner reviewed Apr 19, 2024

View reviewed changes

temporal/api/command/v1/message.proto Show resolved Hide resolved

pdoerner approved these changes Apr 19, 2024

View reviewed changes

rodrigozhou approved these changes Apr 19, 2024

View reviewed changes

cretz reviewed Apr 22, 2024

View reviewed changes

bergundy added 3 commits April 22, 2024 09:11

A couple of docstring fixes

51c424e

Address review comments

fae84d1

Fix indentation

72b95b8

cretz approved these changes Apr 23, 2024

View reviewed changes

temporal/api/command/v1/message.proto Outdated Show resolved Hide resolved

temporal/api/command/v1/message.proto Outdated Show resolved Hide resolved

temporal/api/history/v1/message.proto Outdated Show resolved Hide resolved

bergundy force-pushed the nexus-e2e branch from d1d310d to edc36b0 Compare April 23, 2024 14:18

Apply suggestions from code review

ee2fcbb

bergundy force-pushed the nexus-e2e branch from edc36b0 to ee2fcbb Compare April 23, 2024 14:25

bergundy mentioned this pull request Apr 23, 2024

Nexus E2E implementation temporalio/temporal#5779

Merged

bergundy merged commit 2952cc5 into temporalio:master Apr 23, 2024
3 checks passed

bergundy deleted the nexus-e2e branch April 23, 2024 17:09

	COMMAND_TYPE_CANCEL_NEXUS_OPERATION = 18;
	COMMAND_TYPE_REQUEST_CANCEL_NEXUS_OPERATION = 18;

	NEXUS_OPERATION_STATE_TIMEDOUT = 8;
	NEXUS_OPERATION_STATE_TIMED_OUT = 8;

	EVENT_TYPE_NEXUS_OPERATION_TIMEDOUT = 53;
	EVENT_TYPE_NEXUS_OPERATION_TIMED_OUT = 53;

	NexusOperationCanceledEventAttributes nexus_operation_canceld_event_attributes = 57;
	NexusOperationCanceledEventAttributes nexus_operation_canceled_event_attributes = 57;

	NexusOperationTimedOutEventAttributes nexus_operation_timedout_event_attributes = 58;
	NexusOperationTimedOutEventAttributes nexus_operation_timed_out_event_attributes = 58;

	// The URL should be accessable to the handler of the StartOperation requests.
	// The URL should be accessible to the handler of the StartOperation requests.

Rest of Nexus API definitions #363

Rest of Nexus API definitions #363

Conversation

bergundy commented Mar 6, 2024 • edited Loading

bergundy commented Mar 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cretz Mar 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bergundy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cretz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rodrigozhou left a comment

Choose a reason for hiding this comment

cretz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cretz Apr 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bergundy commented Apr 22, 2024

cretz left a comment

Choose a reason for hiding this comment

bergundy commented Apr 23, 2024

bergundy commented Mar 6, 2024 •

edited

Loading

cretz Mar 7, 2024 •

edited

Loading

cretz Apr 22, 2024 •

edited

Loading