Skip to content

Conversation

@alexandermalyga
Copy link
Contributor

related: #27561

Adds the ttlSecondsAfterFinished parameter to migrateDatabaseJob and createUserJob, so job objects can be automatically deleted after execution.

@boring-cyborg boring-cyborg bot added the area:helm-chart Airflow Helm Chart label Feb 2, 2023
{{- $annotations | toYaml | nindent 4 }}
{{- end }}
spec:
{{- if not (kindIs "invalid" .Values.createUserJob.ttlSecondsAfterFinished) }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checking specifically for not nil because 0 is a valid value for this field and it would evaluate to false normally.

@potiuk
Copy link
Member

potiuk commented Feb 13, 2023

Ah, I missed that one and it's a better fix than mine #29439

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

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)

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

OK. Pending the two small comments @dstandish @jedcunningham - in case you think there is a reason why job name cannot be changed, pleas shout :)

@alexandermalyga
Copy link
Contributor Author

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)

I tried and this affects a lot of tests, let's do it in a separate PR?

@potiuk
Copy link
Member

potiuk commented Feb 13, 2023

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)

I tried and this affects a lot of tests, let's do it in a separate PR?

I am fine with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:helm-chart Airflow Helm Chart

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants