Skip to content

Add updated field on Action struct #129

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.

Sign up for GitHub

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

Jump to bottom
Merged
rabbah merged 2 commits into from
Jan 20, 2020
Merged

Add updated field on Action struct #129

rabbah merged 2 commits into from
Jan 20, 2020

Conversation

@upgle
Copy link
Member

@upgle upgle commented Sep 26, 2019

Description

It is related to this PR apache/openwhisk#4646

Since there is no updated field in the Action structure, wsk action get command doesn't show a updated value even though the API passed it.

I think this PR can be merged after the openwhisk-core PR is merged

TEST

Response body received:
{"annotations":[],"exec":{"kind":"nodejs:10","binary":false},"limits":{"concurrency":1,"logs":10,"memory":128,"timeout":60000},"name":"invokerHealthTestAction0","namespace":"whisk.system","parameters":[],"publish":false,"updated":1569485478259,"version":"0.0.1"}
ok: got action invokerHealthTestAction0
{
    "namespace": "whisk.system",
    "name": "invokerHealthTestAction0",
    "version": "0.0.1",
    "exec": {
        "kind": "nodejs:10",
        "binary": false
    },
    "limits": {
        "timeout": 60000,
        "memory": 128,
        "logs": 10,
        "concurrency": 1
    },
    "publish": false,
    "updated": 1569485478259
}

rabbah
rabbah reviewed Sep 26, 2019
View reviewed changes
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.

You might need to make it an option. Also are tests here in this repo going to be affected when your upstream change is made?

mdeuser
mdeuser previously requested changes Dec 4, 2019
View reviewed changes
whisk/action.go Outdated
Error string `json:"error,omitempty"`
Code int `json:"code,omitempty"`
Publish *bool `json:"publish,omitempty"`
Updated int64 `json:"updated"`
Copy link
Contributor

@mdeuser mdeuser Dec 4, 2019

Choose a reason for hiding this comment

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

for backwards compatibility with existing openwhisk installations i'm thinking that json:"updated,omitempty" should be used.

Copy link
Member Author

@upgle upgle Dec 5, 2019

Choose a reason for hiding this comment

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

Thank you for review. I added an omitempty option for backward compatibility.

@rabbah rabbah dismissed mdeuser’s stale review January 20, 2020 21:21

already addressed.

@rabbah rabbah merged commit 049acfb into apache:master Jan 20, 2020
@rabbah
Copy link
Member

rabbah commented Jan 20, 2020

@upgle should the same change be made to the other entities?

@upgle
Copy link
Member Author

upgle commented Jan 21, 2020

@rabbah yes right, I'll change other entities as well.

@upgle upgle mentioned this pull request Feb 1, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@style95 style95 style95 approved these changes

@rabbah rabbah rabbah approved these changes

@mdeuser mdeuser mdeuser left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@upgle @rabbah @style95 @mdeuser