Miretpl commented on code in PR #71237:
URL: https://github.com/apache/airflow/pull/71237#discussion_r3778672063
##########
chart/newsfragments/71237.significant.rst:
##########
@@ -0,0 +1,5 @@
+PgBouncer Deployment now sets ``terminationGracePeriodSeconds`` (default
``120``)
Review Comment:
I think that this change does not need the newsfragment. Could you delete it?
##########
chart/values.schema.json:
##########
@@ -7801,6 +7801,12 @@
}
]
},
+ "terminationGracePeriodSeconds": {
+ "description": "Grace period for PgBouncer to finish after
SIGTERM is sent from Kubernetes. The default matches the default preStop hook,
which needs up to 120 seconds to drain client connections.",
Review Comment:
```suggestion
"description": "Grace period for PgBouncer to finish
after SIGTERM is sent from Kubernetes.",
```
It is more justification for why we have default on 120 and it is rather not
needed on the parameter documentation level.
##########
chart/values.schema.json:
##########
@@ -7801,6 +7801,12 @@
}
]
},
+ "terminationGracePeriodSeconds": {
+ "description": "Grace period for PgBouncer to finish after
SIGTERM is sent from Kubernetes. The default matches the default preStop hook,
which needs up to 120 seconds to drain client connections.",
+ "type": "integer",
+ "default": 120,
Review Comment:
```suggestion
"default": 120,
"minimum": 0
```
for better validation.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
