You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "alexandermalyga (via GitHub)" <gi...@apache.org> on 2023/02/02 13:09:20 UTC
[GitHub] [airflow] alexandermalyga opened a new pull request, #29314: Add ttlSecondsAfterFinished to migrateDatabaseJob and createUserJob
alexandermalyga opened a new pull request, #29314:
URL: https://github.com/apache/airflow/pull/29314
related: #27561
Adds the [ttlSecondsAfterFinished](https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/) parameter to `migrateDatabaseJob` and `createUserJob`, so job objects can be automatically deleted after execution.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #29314: Add ttlSecondsAfterFinished to migrateDatabaseJob and createUserJob
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #29314:
URL: https://github.com/apache/airflow/pull/29314
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #29314: Add ttlSecondsAfterFinished to migrateDatabaseJob and createUserJob
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #29314:
URL: https://github.com/apache/airflow/pull/29314#discussion_r1104127904
##########
chart/values.yaml:
##########
@@ -759,6 +759,8 @@ scheduler:
# Airflow create user job settings
createUserJob:
+ # Limit the lifetime of the job object after it finished execution.
+ ttlSecondsAfterFinished: ~
Review Comment:
I think we should come up with reasonable default (I had 300 in #29439)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] alexandermalyga commented on pull request #29314: Add ttlSecondsAfterFinished to migrateDatabaseJob and createUserJob
Posted by "alexandermalyga (via GitHub)" <gi...@apache.org>.
alexandermalyga commented on PR #29314:
URL: https://github.com/apache/airflow/pull/29314#issuecomment-1427812068
> This is better than mine - also @hussein-awala suggest that we could rename the jobs so that it will also work even if update is done quickly. See [#29439 (review)](https://github.com/apache/airflow/pull/29439#pullrequestreview-1294842286)
I tried and this affects a lot of tests, let's do it in a separate PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #29314: Add ttlSecondsAfterFinished to migrateDatabaseJob and createUserJob
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29314:
URL: https://github.com/apache/airflow/pull/29314#issuecomment-1427528760
Ach I missed that one and it's a better fix than mine #29439
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #29314: Add ttlSecondsAfterFinished to migrateDatabaseJob and createUserJob
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29314:
URL: https://github.com/apache/airflow/pull/29314#issuecomment-1427815884
> > This is better than mine - also @hussein-awala suggest that we could rename the jobs so that it will also work even if update is done quickly. See [#29439 (review)](https://github.com/apache/airflow/pull/29439#pullrequestreview-1294842286)
>
> I tried and this affects a lot of tests, let's do it in a separate PR?
I am fine with it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] alexandermalyga commented on a diff in pull request #29314: Add ttlSecondsAfterFinished to migrateDatabaseJob and createUserJob
Posted by "alexandermalyga (via GitHub)" <gi...@apache.org>.
alexandermalyga commented on code in PR #29314:
URL: https://github.com/apache/airflow/pull/29314#discussion_r1094788115
##########
chart/templates/jobs/create-user-job.yaml:
##########
@@ -48,6 +48,9 @@ metadata:
{{- $annotations | toYaml | nindent 4 }}
{{- end }}
spec:
+ {{- if not (kindIs "invalid" .Values.createUserJob.ttlSecondsAfterFinished) }}
Review Comment:
Checking specifically for `not nil` because `0` is a valid value for this field and it would evaluate to false normally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org