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

Add checkhealth endpoint #7

Merged
merged 34 commits into from
Jul 22, 2023
Merged

Add checkhealth endpoint #7

merged 34 commits into from
Jul 22, 2023

Conversation

dakennguyen
Copy link
Contributor

@dakennguyen dakennguyen commented Jun 24, 2023

Description

  • Protect function call with circuit breaker:
    • if the circuit is open, a function call will raise "too many failures" error
    • if the circuit is closed, perform the function call will:
      ++ if call successfully, set the circuit status to closed and cache this result for 5 minutes
      ++ if call unsuccessfully, set the circuit status to open and cache this result for 30 seconds
  • Add health endpoint to return UP (the circuit is closed) and DOWN (the circuit is open) status for each model. This endpoint is called by Artermis regularly, so it will try to get the status from the cache before calling to get new status

Step for testing

  • Purposely set wrong config for one model or just use the values from application.example.yml
  • Call POST /api/v1/messages for 3 times for that model, and the fourth time will have "too many failures" error
  • Wait for 30 seconds and can continue to call that model
  • Call GET /api/v1/health and should have the correct status for the models

@@ -7,8 +7,11 @@
class GuidanceWrapper:
"""A wrapper service to all guidance package's methods."""

def __new__(cls, *_, **__):
return super(GuidanceWrapper, cls).__new__(cls)

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 need to add this because when mocking __new__ I got this problem pytest-dev/pytest#8009
If anyone knows a better way to mock, I would be really appreciated :)

@dakennguyen dakennguyen changed the title feature/checkhealth endpoint Add checkhealth endpoint Jun 24, 2023
@dakennguyen dakennguyen marked this pull request as ready for review June 24, 2023 16:01
@dakennguyen dakennguyen requested a review from Hialus June 24, 2023 16:01
@dakennguyen dakennguyen temporarily deployed to Iris Test June 24, 2023 16:02 — with GitHub Actions Inactive
@dakennguyen dakennguyen self-assigned this Jun 24, 2023
@dakennguyen dakennguyen force-pushed the feature/checkhealth-endpoint branch from e86f3db to 3a5eba5 Compare July 6, 2023 19:15
@dakennguyen dakennguyen force-pushed the feature/checkhealth-endpoint branch from 3a5eba5 to 472f3f8 Compare July 6, 2023 19:20
@dakennguyen dakennguyen force-pushed the feature/checkhealth-endpoint branch from 472f3f8 to 0cb68d9 Compare July 6, 2023 19:41
@dakennguyen dakennguyen requested a review from a team July 6, 2023 19:45
@dakennguyen dakennguyen temporarily deployed to Iris Test July 16, 2023 19:56 — with GitHub Actions Inactive
fschoenberger
fschoenberger previously approved these changes Jul 18, 2023
Copy link

@FelixTJDietrich FelixTJDietrich left a comment

Choose a reason for hiding this comment

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

Code mostly LGTM, I just have two remarks.

app/services/cache.py Outdated Show resolved Hide resolved
app/services/guidance_wrapper.py Show resolved Hide resolved
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.

6 participants