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

Hide version in activation's path generated by generateFallbackActivation #5025

Merged
merged 1 commit into from
Nov 11, 2020

Conversation

upgle
Copy link
Member

@upgle upgle commented Nov 10, 2020

Description

If the activation is generated by the generateFallbackActivation method in the InvokerReactive, it contains the version in the path annotation.

In this case, the ES activation store can't search for activations by the name without version.

And also, all other components that generate activation don't store version, but only InvokerReactive stores the version in the path annotation. It must be consistent.

Parameters(WhiskActivation.pathAnnotation, JsString(session.action.fullyQualifiedName(false).asString)) ++

Parameters(WhiskActivation.pathAnnotation, JsString(job.action.fullyQualifiedName(false).asString)) ++

Parameters(WhiskActivation.pathAnnotation, JsString(action.fullyQualifiedName(false).asString)) ++

Path annotation

{
    "annotations": [
        {
            "key": "kind",
            "value": "nodejs:10"
        },
        {
            "key": "path",
            "value": "seonghyun/a2@0.0.154"
        }
    ]
}

Path annotation (expected)

{
    "annotations": [
        {
            "key": "kind",
            "value": "nodejs:10"
        },
        {
            "key": "path",
            "value": "seonghyun/a2"
        }
    ]
}

Related issue and scope

  • None

My changes affect the following components

  • Data stores (activation format)

Types of changes

  • Enhancement or new feature (adds new functionality).

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.

Copy link
Member

@style95 style95 left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-io
Copy link

Codecov Report

Merging #5025 (06fc8b6) into master (cb16450) will decrease coverage by 6.85%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5025      +/-   ##
==========================================
- Coverage   83.77%   76.91%   -6.86%     
==========================================
  Files         202      202              
  Lines        9787     9787              
  Branches      432      432              
==========================================
- Hits         8199     7528     -671     
- Misses       1588     2259     +671     
Impacted Files Coverage Δ
...pache/openwhisk/core/invoker/InvokerReactive.scala 80.00% <ø> (ø)
...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%> (-95.85%) ⬇️
...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%> (-78.58%) ⬇️
...whisk/core/database/cosmosdb/CosmosDBSupport.scala 0.00% <0.00%> (-74.08%) ⬇️
... and 10 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 cb16450...06fc8b6. Read the comment docs.

@rabbah rabbah merged commit 12ca4e3 into apache:master Nov 11, 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.

4 participants