Include triggerer health status in Airflow /health endpoint #31529
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pankajkoti
requested review from
ryanahamilton,
ashb,
bbovenzi,
pierrejeambrun and
ephraimbuddy
as code owners
boring-cyborg
bot
added
area:API
pankajkoti
changed the title
pankajkoti
force-pushed
the
expose-triggerer-status-on-airflow-health-endpoint
branch
from
2b49b61 to
392481a
Compare
2 tasks
|
API responses now: 
|
pankajkoti
force-pushed
the
expose-triggerer-status-on-airflow-health-endpoint
branch
from
392481a to
8bfe5de
Compare
uranusjr
approved these changes
May 25, 2023
4 tasks
pankajkoti
force-pushed
the
expose-triggerer-status-on-airflow-health-endpoint
branch
from
3ff9a49 to
6b7e659
Compare
pierrejeambrun
approved these changes
May 25, 2023
airflow/api/common/airflow_health.py
Outdated
| UNHEALTHY = "unhealthy" | ||
|
|
||
|
|
||
| def get_airflow_health(): |
There was a problem hiding this comment.
Maybe we could add a return type annotation. (TypedDict, or Dict at least ?)
There was a problem hiding this comment.
I tried to be descriptive by specifying the return type as dict[str, dict[str, str | None]], but it failed both locally (I thought my breeze might be acting weird) and also on CI with the below error, so changed it to dict[str, Any] which breeze accepted. So, hopefully CI will accept it too 🤞🏽 .
There was a problem hiding this comment.
yes, CI is green now.
There was a problem hiding this comment.
I think you missed a nested level of dict, but I think any is acceptable here too.
jedcunningham
approved these changes
May 26, 2023
|
1 failing check in CI :( I think this is unrelated, but I will try another rebase to re-trigger CI. 
|
PR apache#27755 introduced sending triggerer health status in the `/api/v1/health` endpoint and also updated relevant docs but we've the primary `/health` too which is missing this information. The PR addresses this missing status report for triggerer health in the `/health` endpoint. It also attempts to deduplicate the code between those endpoints so that in future we need to make necessary changes in only one place and at the same time ensure that change made in one endpoint is not missed for the other endpoint serving the same purpose and thus ensuring consistency in the responses. fixes: apache#31522
pankajkoti
force-pushed
the
expose-triggerer-status-on-airflow-health-endpoint
branch
from
ba47a63 to
22994d7
Compare
jedcunningham
approved these changes
May 26, 2023
jedcunningham
deleted the
expose-triggerer-status-on-airflow-health-endpoint
branch
eladkal
pushed a commit
that referenced
this pull request
Jun 8, 2023
PR #27755 introduced sending triggerer health status in the `/api/v1/health` endpoint and also updated relevant docs but we've the primary `/health` too which is missing this information. The PR addresses this missing status report for triggerer health in the `/health` endpoint. It also attempts to deduplicate the code between those endpoints so that in future we need to make necessary changes in only one place and at the same time ensure that change made in one endpoint is not missed for the other endpoint serving the same purpose and thus ensuring consistency in the responses. fixes: #31522 (cherry picked from commit f048aba)
eladkal
pushed a commit
that referenced
this pull request
Jun 9, 2023
PR #27755 introduced sending triggerer health status in the `/api/v1/health` endpoint and also updated relevant docs but we've the primary `/health` too which is missing this information. The PR addresses this missing status report for triggerer health in the `/health` endpoint. It also attempts to deduplicate the code between those endpoints so that in future we need to make necessary changes in only one place and at the same time ensure that change made in one endpoint is not missed for the other endpoint serving the same purpose and thus ensuring consistency in the responses. fixes: #31522 (cherry picked from commit f048aba)
62 tasks
This was referenced Jun 30, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR #27755 introduced sending triggerer health status in the
/api/v1/healthendpoint and also updated relevant docs but we have the primary
/healthendpoint too which is missing this information. The PR addresses this missing
status report for triggerer health in the
/healthendpoint. It also attempts todeduplicate the code between those endpoints so that in future when we need
to make necessary changes in only one place and at the same time ensure that
change made in one endpoint is not missed for the other endpoint serving the
same purpose and thus ensuring consistency in the responses.
fixes: #31522
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.