Skip to content

Infrequent polling benign exception#42

Merged
THardy98 merged 8 commits intomainfrom
infrequent_polling_benign_exception
Jul 28, 2025
Merged

Infrequent polling benign exception#42
THardy98 merged 8 commits intomainfrom
infrequent_polling_benign_exception

Conversation

@THardy98
Copy link
Copy Markdown
Contributor

What was changed

Added infrequent polling sample that uses BENIGN category for the application failure

Why?

Sample parity + demonstrate usage of BENIGN

@THardy98 THardy98 requested a review from a team as a code owner July 25, 2025 13:47
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jul 25, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Contributor

@cretz cretz left a comment

Choose a reason for hiding this comment

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

Thanks! If you have the available time, to reach real parity would be to add the other two polling samples so we can close #12, but if on limited time or want to handle that elsewhere, no problem

Comment thread polling/infrequent/compose_greeting_activity.rb Outdated
Comment thread polling/infrequent/test_service.rb Outdated
Comment thread polling/infrequent/test_service.rb Outdated
Comment thread polling/infrequent/README.md
Comment thread Gemfile.lock
Comment thread polling/infrequent/test_service.rb Outdated
@THardy98 THardy98 requested a review from cretz July 25, 2025 15:10
Copy link
Copy Markdown
Contributor

@cretz cretz left a comment

Choose a reason for hiding this comment

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

Looking good, only minor things. Would be nice if there was a test to ensure this sample keeps working, but not technically required.

Comment thread README.md Outdated
Comment thread polling/infrequent/greeting_workflow.rb Outdated
@THardy98
Copy link
Copy Markdown
Contributor Author

Looking good, only minor things. Would be nice if there was a test to ensure this sample keeps working, but not technically required.

added a test :)

@THardy98 THardy98 requested a review from cretz July 25, 2025 16:25
@cretz
Copy link
Copy Markdown
Contributor

cretz commented Jul 25, 2025

added a test :)

Missing a push?

@THardy98
Copy link
Copy Markdown
Contributor Author

added a test :)

Missing a push?

yup 🙃 sorry

Copy link
Copy Markdown
Contributor

@cretz cretz left a comment

Choose a reason for hiding this comment

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

Nothing blocking, though to pass CI may need to fix Rubocop issues and skip time-skipping test in certain environments.

Comment thread test/polling/infrequent_polling_test.rb Outdated
Comment thread test/polling/infrequent_polling_test.rb Outdated
def test_workflow_completes_after_polling
task_queue = "tq-#{SecureRandom.uuid}"

Temporalio::Testing::WorkflowEnvironment.start_time_skipping do |env|
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure if all environments will support time skipping. In our Ruby SDK base Test class we have:

  def skip_if_not_x86!
    skip('Test only supported on x86') unless RbConfig::CONFIG['host_cpu'] == 'x86_64'
  end

And we invoke that at the top of any test that needs time skipping. May want to do the same here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added to the best test.rb file that the tests inherit from, added to test

@THardy98 THardy98 merged commit 410f2d5 into main Jul 28, 2025
8 checks passed
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.

3 participants