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.

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

feat(client): add client.workflow.count high level API #1573

Merged
merged 9 commits into from
Dec 9, 2024

Conversation

THardy98
Copy link
Contributor

@THardy98 THardy98 commented Nov 21, 2024

What was changed

Added a count method to the workflow client, a higher-level, user-friendlier option to using the corresponding GRPC method directly.

  1. Closes [Feature Request] Support countWorkflowExecutions #1069

  2. How was this tested:
    Added an integration test.

  3. Any docs updates needed?
    Not sure

@THardy98 THardy98 requested a review from a team as a code owner November 21, 2024 17:55
@THardy98 THardy98 marked this pull request as draft November 21, 2024 17:55
@THardy98 THardy98 requested a review from mjameswh November 21, 2024 17:55
@THardy98 THardy98 changed the title [DRAFT] Support countWorkflowExecutions for workflow client feat(client): add client.workflow.count high level API Nov 21, 2024
@THardy98 THardy98 marked this pull request as ready for review November 21, 2024 23:28
@THardy98 THardy98 requested a review from mjameswh November 21, 2024 23:40
@@ -1,7 +1,8 @@
import { randomUUID } from 'crypto';
import { ExecutionContext } from 'ava';
import { firstValueFrom, Subject } from 'rxjs';
import { WorkflowFailedError } from '@temporalio/client';
import asyncRetry from 'async-retry';
Copy link
Contributor

Choose a reason for hiding this comment

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

There are some lint warnings here. Make sure to execute npm run lint before pushing to GitHub.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd run this prior to pushing, does it not make code changes, just warnings?
In any case, I've addressed the warnings.

Copy link
Contributor

Choose a reason for hiding this comment

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

npm run lint should fix the code, if auto-fix is possible. Not sure why it didn't.

@THardy98 THardy98 requested a review from mjameswh November 22, 2024 18:47
@THardy98 THardy98 force-pushed the support-countWorkflowExecutions branch from 345d700 to 40489d8 Compare November 22, 2024 19:13
@THardy98 THardy98 requested a review from mjameswh November 25, 2024 19:50
Copy link
Contributor

@mjameswh mjameswh left a comment

Choose a reason for hiding this comment

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

Great! Thanks a lot, @THardy98!

@mjameswh mjameswh merged commit 53233c9 into main Dec 9, 2024
23 checks passed
@mjameswh mjameswh deleted the support-countWorkflowExecutions branch December 9, 2024 22:40
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.

[Feature Request] Support countWorkflowExecutions
2 participants