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

Always return activation without log on blocking invoke. #4100

Merged
merged 1 commit into from
Nov 6, 2018

Conversation

cbickel
Copy link
Contributor

@cbickel cbickel commented Nov 6, 2018

If an user invokes an action blockingly today, the invoker executes the activation and sends back the result to the controller, which passes it back to the client. If it is not possible to pass the result back to the controller, it will try to get it from the database. And here's the problem. If the controller receives the result of the activation from the invoker, there are no logs included. If it polls it from the database, the logs are included. This should be unified to never return logs.

Related issue and scope

  • I opened an issue to propose and discuss this change (#????)

My changes affect the following components

  • API
  • Controller
  • Message Bus (e.g., Kafka)
  • Loadbalancer
  • Invoker
  • Intrinsic actions (e.g., sequences, conductors)
  • Data stores (e.g., CouchDB)
  • Tests
  • Deployment
  • CLI
  • General tooling
  • Documentation

Types of changes

  • Bug fix (generally a non-breaking change which closes an issue).
  • 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.

If an user invokes an action blockingly today, the invoker executes the activation and sends back the result to the controller, which passes it back to the client. If it is not possible to pass the result back to the controller, it will try to get it from the database. And here's the problem. If the controller receives the result of the activation from the invoker, there are no logs included. If it polls it from the database, the logs are included. This should be unified to never return logs.
@cbickel cbickel added the review Review for this PR has been requested and yet needs to be done. label Nov 6, 2018
Copy link
Contributor

@markusthoemmes markusthoemmes left a comment

Choose a reason for hiding this comment

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

Makes sense and LGTM 👍

@codecov-io
Copy link

Codecov Report

Merging #4100 into master will increase coverage by 4.59%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4100      +/-   ##
==========================================
+ Coverage   76.63%   81.22%   +4.59%     
==========================================
  Files         148      148              
  Lines        7249     7249              
  Branches      445      442       -3     
==========================================
+ Hits         5555     5888     +333     
+ Misses       1694     1361     -333
Impacted Files Coverage Δ
...isk/core/controller/actions/PrimitiveActions.scala 87.5% <100%> (+2.08%) ⬆️
...core/database/cosmosdb/RxObservableImplicits.scala 0% <0%> (-100%) ⬇️
...core/database/cosmosdb/CosmosDBArtifactStore.scala 0% <0%> (-95.54%) ⬇️
...sk/core/database/cosmosdb/CosmosDBViewMapper.scala 0% <0%> (-92.6%) ⬇️
...whisk/core/database/cosmosdb/CosmosDBSupport.scala 0% <0%> (-83.34%) ⬇️
...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala 0% <0%> (-58.83%) ⬇️
...la/whisk/core/database/cosmosdb/CosmosDBUtil.scala 92% <0%> (-4%) ⬇️
...r/src/main/scala/whisk/core/controller/Rules.scala 89.93% <0%> (+0.67%) ⬆️
...src/main/scala/whisk/core/controller/Actions.scala 93.03% <0%> (+1.49%) ⬆️
.../containerpool/ApacheBlockingContainerClient.scala 72.72% <0%> (+1.81%) ⬆️
... and 42 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 f47410d...a6e71b5. Read the comment docs.

@rabbah
Copy link
Member

rabbah commented Nov 6, 2018

LGTM also - I thought we used to do this in the Actions API route but must have lost it in some refactoring. Good catch.

@rabbah rabbah merged commit 729cb2c into apache:master Nov 6, 2018
@cbickel cbickel deleted the ack branch November 6, 2018 12:46
BillZong pushed a commit to BillZong/openwhisk that referenced this pull request Nov 18, 2019
If a user invokes a blocking action today, the invoker executes the activation and sends back the result to the controller, which passes it back to the client. If it is not possible to pass the result back to the controller, the controller will try to get the result from the database. And here's the problem. If the controller receives the result of the activation from the invoker, there are no logs included. If it polls it from the database, the logs are included. This should be unified to never return logs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Review for this PR has been requested and yet needs to be done.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants