Serialize `updated` value of entity document in response by upgle · Pull Request #4646 · apache/openwhisk

upgle · 2019-09-26T06:05:58Z

Description

Since an action API doesn't serialize the updated value of the action entity, users can't know when the action was updated.

It changes the action API to serialize and provide the updated value of the action

Related issue and scope

#4228

My changes affect the following components

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.

rabbah

Why only actions? All assets have an updated field.

upgle · 2019-09-27T06:50:44Z

@rabbah Thank you for review. I think the updated field of activation doesn't need to be serialized because it has start and end timestamp fields.

So I'll modify all assets (trigger, rules, and package) except activation to serialize updated field.

codecov-io · 2019-09-30T06:23:03Z

Codecov Report

Merging #4646 into master will decrease coverage by 6.66%.
The diff coverage is 92.3%.

@@            Coverage Diff             @@
##           master    #4646      +/-   ##
==========================================
- Coverage   84.99%   78.32%   -6.67%     
==========================================
  Files         198      198              
  Lines        8845     8837       -8     
  Branches      615      606       -9     
==========================================
- Hits         7518     6922     -596     
- Misses       1327     1915     +588

Impacted Files	Coverage Δ
...org/apache/openwhisk/core/entity/WhiskEntity.scala	`92.3% <100%> (+0.12%)`	⬆️
...rg/apache/openwhisk/core/entity/WhiskTrigger.scala	`100% <100%> (ø)`	⬆️
...rg/apache/openwhisk/core/entity/WhiskPackage.scala	`96.77% <100%> (ø)`	⬆️
...a/org/apache/openwhisk/core/entity/WhiskRule.scala	`82.35% <100%> (ø)`	⬆️
...org/apache/openwhisk/core/entity/WhiskAction.scala	`91.25% <83.33%> (+0.09%)`	⬆️
...core/database/cosmosdb/RxObservableImplicits.scala	`0% <0%> (-100%)`	⬇️
...enwhisk/connector/kafka/KamonMetricsReporter.scala	`0% <0%> (-100%)`	⬇️
...e/database/cosmosdb/cache/ChangeFeedConsumer.scala	`0% <0%> (-100%)`	⬇️
...ore/database/cosmosdb/cache/CacheInvalidator.scala	`0% <0%> (-100%)`	⬇️
...core/database/cosmosdb/CosmosDBArtifactStore.scala	`0% <0%> (-96.23%)`	⬇️
... and 24 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 3235a94...26a446f. Read the comment docs.

upgle · 2019-09-30T06:25:39Z

It passed all the tests 👍

rabbah · 2019-11-11T15:02:48Z

  val publish: Boolean
  val annotations: Parameters
-  val updated = Instant.now(Clock.systemUTC())
+  val updated = WhiskEntity.currentMillis()


since all the subtypes now assign this value in the constructors, can we make this an abstract value?

I think we can't make this an abstract value because the WhiskActivation entity type doesn't assign this value to the constructor.

rabbah · 2019-11-12T13:43:02Z

Changes lgtm now with the added explanation.

rabbah · 2019-11-13T14:01:09Z

  def aname() = MakeName.next("packages_tests")
  val parametersLimit = Parameters.sizeLimit

+  def checkResponse(response: WhiskPackage, expected: WhiskPackage) =


kudos - thanks for doing this.

tysonnorris · 2019-11-13T16:01:25Z

Looks like a failure in RulesApiTests

upgle · 2019-11-14T01:49:14Z

One test case has been broken by refactoring. I just fixed it.

style95

LGTM

My comment would not block merging this.

style95 · 2019-11-15T02:16:56Z

  val parametersLimit = Parameters.sizeLimit
+  val dummyInstant = Instant.now()
+
+  def checkResponse(response: WhiskRuleResponse, expected: WhiskRuleResponse) =


One small nit can be extracting this kind of methods into a trait.

@style95 Thank you for review, I've created a common method to test the WhiskAction, WhiskActionMetaData, WhiskTrigger, and WhiskPackage. But I couldn't extract the method in the RulesApiTests because WhiskRuleResponse doesn't extend WhiskEntity.

Please review it again 😄

26a446f#diff-a151ad71ab374ea12d3b9c780b59de30R88

codecov-commenter · 2024-10-14T22:14:14Z

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 78.32%. Comparing base (c674757) to head (26a446f).
Report is 445 commits behind head on master.

Files with missing lines	Patch %	Lines
...org/apache/openwhisk/core/entity/WhiskAction.scala	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4646      +/-   ##
==========================================
+ Coverage   77.92%   78.32%   +0.40%     
==========================================
  Files         198      198              
  Lines        8841     8837       -4     
  Branches      614      606       -8     
==========================================
+ Hits         6889     6922      +33     
+ Misses       1952     1915      -37

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

upgle changed the title ~~Serialize the updated value in the db out in the action~~ Serialize updated value in action doc Sep 26, 2019

upgle mentioned this pull request Sep 26, 2019

Add updated field on Action struct apache/openwhisk-client-go#129

Merged

rabbah reviewed Sep 26, 2019

View reviewed changes

Comment thread common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala Outdated

Comment thread tests/src/test/scala/org/apache/openwhisk/core/controller/test/ActionsApiTests.scala Outdated

upgle force-pushed the feature/add-updated-field branch from 235b568 to e44dd6c Compare September 27, 2019 07:58

upgle changed the title ~~Serialize updated value in action doc~~ Serialize updated value of entity document in response Sep 27, 2019

upgle force-pushed the feature/add-updated-field branch from 13cf028 to b09d783 Compare September 30, 2019 01:39

upgle requested a review from rabbah September 30, 2019 06:27

rabbah reviewed Oct 30, 2019

View reviewed changes

Comment thread common/scala/src/main/scala/org/apache/openwhisk/core/entity/WhiskAction.scala

rabbah reviewed Nov 12, 2019

View reviewed changes

rabbah approved these changes Nov 13, 2019

View reviewed changes

upgle force-pushed the feature/add-updated-field branch 2 times, most recently from 3971288 to 1606a42 Compare November 14, 2019 08:39

upgle added 12 commits November 15, 2019 10:41

Add updated field on WhiskAction json format

f8c182d

Update custom serdes

df3852d

Add updated field at case class

82bdc65

Fix package api test

776a27d

Modified test case name

2e5bd4a

Add annotation

00dddf6

Add singleton method to get current timestamp

f5b8a92

Modify test case

7a56f53

Update annotation

95fadfc

Add updated field on package entity

9d66af7

Add updated field on trigger entity

36079f3

Serialize updated field in rules entity

33fa090

upgle added 4 commits November 15, 2019 10:41

Fix scheme tests

2cdf7eb

Update apiv1swagger.json

fda58eb

Refactor test code

348ac8f

Fix test case

2f8571b

upgle force-pushed the feature/add-updated-field branch from 1606a42 to 2f8571b Compare November 15, 2019 02:06

Fix typo

0fa89f8

style95 approved these changes Nov 15, 2019

View reviewed changes

Refactor test code

26a446f

style95 approved these changes Nov 29, 2019

View reviewed changes

style95 merged commit 58c8976 into apache:master Nov 29, 2019

upgle mentioned this pull request Feb 1, 2020

Add updated field on package, rule, and trigger entity apache/openwhisk-client-go#135

Merged

Conversation

upgle commented Sep 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issue and scope

My changes affect the following components

Types of changes

Checklist:

Uh oh!

rabbah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

upgle commented Sep 27, 2019

Uh oh!

codecov-io commented Sep 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

upgle commented Sep 30, 2019

Uh oh!

Uh oh!

rabbah Nov 11, 2019

Choose a reason for hiding this comment

Uh oh!

upgle Nov 13, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rabbah commented Nov 12, 2019

Uh oh!

rabbah Nov 13, 2019

Choose a reason for hiding this comment

Uh oh!

tysonnorris commented Nov 13, 2019

Uh oh!

upgle commented Nov 14, 2019

Uh oh!

style95 left a comment

Choose a reason for hiding this comment

Uh oh!

style95 Nov 15, 2019

Choose a reason for hiding this comment

Uh oh!

upgle Nov 29, 2019

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Oct 14, 2024

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

upgle commented Sep 26, 2019 •

edited

Loading

codecov-io commented Sep 30, 2019 •

edited

Loading