Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Expose count_workflow_executions on the temporal client #272

Merged
merged 8 commits into from
Nov 9, 2023
Merged

Expose count_workflow_executions on the temporal client #272

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
09eca23
Expose count_workflow_executions on the temporal client
harsh-stripe Oct 17, 2023
f8dd685
Return a wrapped type for count_workflows response
harsh-stripe Oct 18, 2023
17660a3
Add integration tests
harsh-stripe Oct 19, 2023
be52fa7
Remove debug log lines
harsh-stripe Oct 19, 2023
35bad5d
Update lib/temporal/client.rb
harsh-stripe Oct 27, 2023
29e7b78
Return the count value of count_workflow_executions instead of wrappi…
harsh-stripe Nov 2, 2023
cbf1008
Resolve merge conflicts
harsh-stripe Nov 2, 2023
3239f9a
Remove the example integration spec for count_workflows. ES isn't set…
harsh-stripe Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/temporal.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ module Temporal
:list_open_workflow_executions,
:list_closed_workflow_executions,
:query_workflow_executions,
:count_workflow_executions,
:add_custom_search_attributes,
:list_custom_search_attributes,
:remove_custom_search_attributes,
Expand Down
6 changes: 6 additions & 0 deletions lib/temporal/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require 'temporal/activity/async_token'
require 'temporal/workflow'
require 'temporal/workflow/context_helpers'
require 'temporal/workflow/count_workflows_aggregation'
require 'temporal/workflow/history'
require 'temporal/workflow/execution_info'
require 'temporal/workflow/executions'
Expand Down Expand Up @@ -425,6 +426,11 @@ def query_workflow_executions(namespace, query, filter: {}, next_page_token: nil
Temporal::Workflow::Executions.new(connection: connection, status: :all, request_options: { namespace: namespace, query: query, next_page_token: next_page_token, max_page_size: max_page_size }.merge(filter))
end

def count_workflow_executions(namespace, query)
Copy link
Contributor

@jazev-stripe jazev-stripe Oct 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

it would be nice to have a short doc-comment with the types of the parameters/return value (even if it isn't enforced without Sorbet, and in this case the types are fairly trivial). I know a lot of the other methods don't have it, but I'm putting my type evangelist hat on 🤠.

response = connection.count_workflow_executions(namespace: namespace, query: query)
Temporal::Workflow::CountWorkflowAggregation.new(count: response.count)
end

# @param attributes [Hash[String, Symbol]] name to symbol for type, see INDEXED_VALUE_TYPE above
# @param namespace String, required for SQL enhanced visibility, ignored for elastic search
def add_custom_search_attributes(attributes, namespace: nil)
Expand Down
11 changes: 11 additions & 0 deletions lib/temporal/workflow/count_workflows_aggregation.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module Temporal
class Workflow
class CountWorkflowAggregation
Copy link
Contributor

Choose a reason for hiding this comment

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

ooc, why "Aggregation" as the suffix instead of a more neutral "Result" or "Response"? Does aggregation mean it is aggregating all of the fields in the CountWorkflowExecutionsResponse, or is it a reference to the CountWorkflowExecutionsResponse.AggregationGroup type?

def initialize(count:)
@count = count
end

attr_reader :count
end
end
end
24 changes: 24 additions & 0 deletions spec/unit/lib/temporal/client_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1102,4 +1102,28 @@ class NamespacedWorkflow < Temporal::Workflow
end
end
end

describe '#count_workflow_executions' do
let(:response) do
Temporalio::Api::WorkflowService::V1::CountWorkflowExecutionsResponse.new(
count: 5
)
end

before do
allow(connection)
.to receive(:count_workflow_executions)
.and_return(response)
end

it 'returns the count' do
resp = subject.count_workflow_executions(namespace, '')
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: having a non-empty query makes the test slightly more realistic (at least I don't think an empty query works on the real server)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Believe it or not, an empty query does work on the real server (see slack), but I've gone ahead and modified the test to be more realistic (there's very few scenarios during which you'd do an empty query).


expect(connection)
.to have_received(:count_workflow_executions)
.with(namespace: namespace, query: '')

expect(resp.count).to eq(5)
end
end
end