Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to get activation list by a binding package name #4919

Merged
merged 5 commits into from
Jul 29, 2020

Conversation

upgle
Copy link
Member

@upgle upgle commented Jun 12, 2020

Description

Users cannot query the activation list by a binding package name. And It also becomes ambiguous if there is a package that is the same as the original package name of the binding package.

This PR changes the activation list query to more clear.

Ambiguous case

  • /whisk.system/p1 (package, shared=yes)
    • a1 (action)
  • /guest/p1 (general package) -> It's using same name as the above shared package
    • a1 (action)
  • /guest/p2 (binding package, /whisk.system/p1)
  • /guest/p3 (binding package, /whisk.system/p1)

In this case, the guest namespace cannot get the activations invoked only in the /guest/p1 package.

The entity name /whisk.system/p1/a1 and /guest/p1/a1 returns same results:

$ wsk activation list /whisk.system/p1/a1
Datetime            Activation ID                    Kind      Start Duration   Status  Entity
2020-06-12 13:18:19 692adad6d83d4172aadad6d83da17207 nodejs:10 cold  2.311s     success guest/a1:0.0.1
2020-06-12 13:17:39 f0223c6f2b2a4383a23c6f2b2af383cb nodejs:10 cold  2.856s     success guest/a1:0.0.1
$ wsk activation list /guest/p1/a1
Datetime            Activation ID                    Kind      Start Duration   Status  Entity
2020-06-12 13:18:19 692adad6d83d4172aadad6d83da17207 nodejs:10 cold  2.311s     success guest/a1:0.0.1
2020-06-12 13:17:39 f0223c6f2b2a4383a23c6f2b2af383cb nodejs:10 cold  2.856s     success guest/a1:0.0.1

User expected behavior for activations of binding package action

In general, the behavior expected by the user is to get activations for each binding package. They expect to see activations of the two binding packages separately.

  • /guest/p2 (binding package)
  • /guest/p3 (binding package)
$ wsk activation list /guest/p2/a1
2020-06-12 14:01:02 2853df16a7f74e9b93df16a7f79e9bc9 nodejs:10 warm  32ms       success guest/a1:0.0.1
$ wsk activation list /guest/p3/a1
2020-06-12 14:00:55 39b688aaea454884b688aaea456884c5 nodejs:10 cold  2.964s     success guest/a1:0.0.1

Related issue and scope

  • None

My changes affect the following components

  • API
  • Data stores (e.g., CouchDB)

Types of changes

  • Enhancement or new feature (adds new functionality).
  • Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

  • I signed an Apache CLA.
  • I reviewed the style guides and followed the recommendations (Travis CI will check :).
  • I added tests to cover my changes.
  • My changes require further changes to the documentation.
  • I updated the documentation where necessary.

@upgle upgle changed the title Allow binding package name filter for wsk activation list Allow to get activation list by a binding package name Jun 12, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jun 12, 2020

Codecov Report

Merging #4919 into master will decrease coverage by 5.79%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4919      +/-   ##
==========================================
- Coverage   83.51%   77.72%   -5.80%     
==========================================
  Files         201      201              
  Lines        9441     9521      +80     
  Branches      396      407      +11     
==========================================
- Hits         7885     7400     -485     
- Misses       1556     2121     +565     
Impacted Files Coverage Δ
...e/elasticsearch/ElasticSearchActivationStore.scala 83.95% <100.00%> (+0.30%) ⬆️
...core/database/cosmosdb/RxObservableImplicits.scala 0.00% <0.00%> (-100.00%) ⬇️
...ore/database/cosmosdb/cache/CacheInvalidator.scala 0.00% <0.00%> (-100.00%) ⬇️
...e/database/cosmosdb/cache/ChangeFeedConsumer.scala 0.00% <0.00%> (-100.00%) ⬇️
...core/database/cosmosdb/CosmosDBArtifactStore.scala 0.00% <0.00%> (-96.23%) ⬇️
...sk/core/database/cosmosdb/CosmosDBViewMapper.scala 0.00% <0.00%> (-93.90%) ⬇️
...tabase/cosmosdb/cache/CacheInvalidatorConfig.scala 0.00% <0.00%> (-92.31%) ⬇️
...enwhisk/connector/kafka/KamonMetricsReporter.scala 0.00% <0.00%> (-83.34%) ⬇️
...e/database/cosmosdb/cache/KafkaEventProducer.scala 0.00% <0.00%> (-77.78%) ⬇️
...whisk/core/database/cosmosdb/CosmosDBSupport.scala 0.00% <0.00%> (-74.08%) ⬇️
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1274fdf...debe388. Read the comment docs.

@style95
Copy link
Member

style95 commented Jun 14, 2020

This would fix #1843.

@@ -3,7 +3,7 @@
"language": "javascript",
"views": {
"activations": {
"map": "function (doc) {\n var PATHSEP = \"/\";\n var isActivation = function (doc) { return (doc.activationId !== undefined) };\n var summarize = function (doc) {\n var endtime = doc.end !== 0 ? doc.end : undefined;\n return {\n namespace: doc.namespace,\n name: doc.name,\n version: doc.version,\n publish: doc.publish,\n annotations: doc.annotations,\n activationId: doc.activationId,\n start: doc.start,\n end: endtime,\n duration: endtime !== undefined ? endtime - doc.start : undefined,\n cause: doc.cause,\n statusCode: (endtime !== undefined && doc.response !== undefined && doc.response.statusCode !== undefined) ? doc.response.statusCode : undefined\n }\n };\n\n var pathFilter = function(doc) {\n for (i = 0; i < doc.annotations.length; i++) {\n var a = doc.annotations[i];\n if (a.key == \"path\") try {\n var p = a.value.split(PATHSEP);\n if (p.length == 3) {\n return p[1] + PATHSEP + doc.name;\n } else return doc.name;\n } catch (e) {\n return doc.name;\n }\n }\n return doc.name;\n }\n\n if (isActivation(doc)) try {\n var value = summarize(doc)\n emit([doc.namespace+PATHSEP+pathFilter(doc), doc.start], value);\n } catch (e) {}\n}\n",
"map": "function (doc) {\n var PATHSEP = \"/\";\n var isActivation = function (doc) { return (doc.activationId !== undefined) };\n var summarize = function (doc) {\n var endtime = doc.end !== 0 ? doc.end : undefined;\n return {\n namespace: doc.namespace,\n name: doc.name,\n version: doc.version,\n publish: doc.publish,\n annotations: doc.annotations,\n activationId: doc.activationId,\n start: doc.start,\n end: endtime,\n duration: endtime !== undefined ? endtime - doc.start : undefined,\n cause: doc.cause,\n statusCode: (endtime !== undefined && doc.response !== undefined && doc.response.statusCode !== undefined) ? doc.response.statusCode : undefined\n }\n };\n \n var pathFilter = function(doc) {\n var path = undefined;\n var binding = undefined;\n for (i = 0; i < doc.annotations.length; i++) {\n var a = doc.annotations[i];\n if (a.key == \"path\") {\n path = a.value;\n }\n if (a.key == \"binding\") {\n binding = a.value;\n }\n }\n try {\n if (binding) {\n var b = binding.split(PATHSEP)\n return b[1] + PATHSEP + doc.name;\n }\n if (path) {\n var p = path.split(PATHSEP);\n if (p.length == 3) {\n return p[1] + PATHSEP + doc.name;\n } else return doc.name; \n }\n } catch (e) {\n return doc.name;\n }\n return doc.name;\n }\n\n if (isActivation(doc)) try {\n var value = summarize(doc)\n emit([doc.namespace+PATHSEP+pathFilter(doc), doc.start], value);\n } catch (e) {}\n}\n",
Copy link
Member

Choose a reason for hiding this comment

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

please use a new view with a different version number since updating this design doc will cause the view to recompute which can take a long time.

Copy link
Member

@rabbah rabbah left a comment

Choose a reason for hiding this comment

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

Thanks for making the change. I also suggest keeping the v2.1.0 view around for a while to allow operators a chance to migrate.

@upgle
Copy link
Member Author

upgle commented Jul 1, 2020

@rabbah Thank you for your review. I've added the v2.1.0 view again.

@style95
Copy link
Member

style95 commented Jul 29, 2020

I am merging this with the confirmation from @rabbah again on Slack.

@style95 style95 merged commit 4921a0e into apache:master Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants