rabbah commented on a change in pull request #5169:
URL: https://github.com/apache/openwhisk/pull/5169#discussion_r787237119
##########
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/database/ActivationStore.scala
##########
@@ -183,17 +191,29 @@ trait ActivationStore {
* - an activation in debug mode
* - activation stores is not disabled via a configuration parameter
*
- * @param isSuccess is successful activation
+ * @param activation to check
* @param isBlocking is blocking activation
* @param debugMode is logging header set to "on" for the invocation
- * @param disableStore is disable store configured
+ * @param blockingStoreLevel level of activation status to store for
blocking invocations
+ * @param nonBlockingStoreLevel level of activation status to store for
blocking invocations
* @return Should the activation be stored to the database
*/
- private def shouldStoreActivation(isSuccess: Boolean,
+ private def shouldStoreActivation(activation: WhiskActivation,
isBlocking: Boolean,
debugMode: Boolean,
- disableStore: Boolean): Boolean = {
- !isSuccess || !isBlocking || debugMode || !disableStore
+ blockingStoreLevel:
ActivationStoreLevel.Value,
+ nonBlockingStoreLevel:
ActivationStoreLevel.Value): Boolean = {
+ def shouldStoreOnLevel(storageLevel: ActivationStoreLevel.Value): Boolean
= {
+ storageLevel match {
+ case ActivationStoreLevel.STORE_ALWAYS => true
+ case ActivationStoreLevel.STORE_FAILURES =>
!activation.response.isSuccess
+ case ActivationStoreLevel.STORE_FAILURES_NOT_APPLICATION_ERRORS =>
+ !activation.response.isSuccess &&
!activation.response.isApplicationError
Review comment:
how about turning this into the two failure modes instead to avoid
potential double negations.
##########
File path:
common/scala/src/main/scala/org/apache/openwhisk/core/database/ActivationStore.scala
##########
@@ -183,17 +191,29 @@ trait ActivationStore {
* - an activation in debug mode
* - activation stores is not disabled via a configuration parameter
*
- * @param isSuccess is successful activation
+ * @param activation to check
* @param isBlocking is blocking activation
* @param debugMode is logging header set to "on" for the invocation
- * @param disableStore is disable store configured
+ * @param blockingStoreLevel level of activation status to store for
blocking invocations
+ * @param nonBlockingStoreLevel level of activation status to store for
blocking invocations
* @return Should the activation be stored to the database
*/
- private def shouldStoreActivation(isSuccess: Boolean,
+ private def shouldStoreActivation(activation: WhiskActivation,
Review comment:
you don't need the entire activation, just the activation.response so
I'd tighten this param's type.
--
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]
