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

Fix health check attributes #519

Merged
merged 2 commits into from
Jun 1, 2021

Conversation

GreyTeardrop
Copy link
Contributor

@GreyTeardrop GreyTeardrop commented May 29, 2021

What was changed

This PR fixes small typos and semantic issues with the health check feature introduced in version 1.0.8.

  1. It seems like the property named disableHealthCheck actually enabled health checks. The HealthCheckTest used to pass as the service.checkHealth("IncorrectServiceName") method did nothing when disableHealthCheck was false.
  2. There's a typo in WorkflowServiceStubsOptions.Builder's method name for healthCheckAttemptTimeout.
  3. WorkflowServiceStubsOptions checks wrong attribute for null before assigning default value for healthCheckAttemptTimeout.
  4. In-memory service does not support health checks so TestWorkflowService, TestWorkflowEnvironmentInternal, and TestActivityEnvironmentInternal have to disable it.

Why?

I've noticed these issues while exposing new attributes added by 1.0.8 as Spring Boot application properties.

Checklist

  1. Closes issue: -

  2. How was this tested: updated unit test.

  3. Any docs updates needed? Javadocs are updated with proper semantics of the disableHealthCheck attribute.

* let disableHealthCheck actually disable health checks
* fix WorkflowServiceStubsOptions.Builder's method name for
  healthCheckAttemptTimeout
@vitarb vitarb merged commit a25cafa into temporalio:master Jun 1, 2021
@GreyTeardrop GreyTeardrop deleted the fix-healthcheck-options branch June 1, 2021 18:21
robzienert pushed a commit to robzienert/sdk-java that referenced this pull request Jul 23, 2021
…LLEY/nflx-temporal-sdk:merge_oss to master

* commit '671858b5039cbce290427ddcf723149c6e60cdd1':
  Small tweak to upgrade notes
  Document upgrade
  Comment out changes that pull in dependencies from grpc 1.34+
  Add netflix.io.grpc:grpc-services-nflx
  update dependency locks
  Implement configurable DEFAULT_DEADLOCK_DETECTION_TIMEOUT (temporalio#524)
  Change InterruptedException logs to debug-level. (temporalio#533)
  Updated dependencies. (temporalio#532)
  Fixed flaky DetachedScopeTest + Javadoc + Refactoring (temporalio#526)
  Add getWorker method to TestWorkflowRule (temporalio#528)
  Avoid throwing IllegalStateException in NoopSuspendableWorker (temporalio#527)
  Cleaning up Activity Javadocs (temporalio#453)
  Add workerFactoryOptions override to the JUnit 5 extension (temporalio#520)
  Removes slf4j-simple from api scope (temporalio#523)
  Release v1.0.9 (temporalio#522)
  Fix health check attributes (temporalio#519)
  Release v1.0.8 (temporalio#518)
  Out-of-process test server (temporalio#470)
  Health Check (temporalio#504)
  Workflow retry in Test Service (temporalio#510)
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.

2 participants