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

Add "current-since" field to services API and CLI #115

Merged
merged 4 commits into from
Dec 5, 2022

Conversation

benhoyt
Copy link
Contributor

@benhoyt benhoyt commented Apr 28, 2022

This PR adds a current-since field to the API, meaning the time at which the "current" field last changed, for example started (current changed from inactive to active), and so on. It's related to #104 (though in a previous iteration it was "start time", not "curren since").

Here's how it looks like the CLI:

$ pebble services
Service  Startup  Current  Since
test2    enabled  active   today at 17:08 NZST

$ pebble services --abs-time
Service  Startup  Current  Since
test2    enabled  active   2022-04-28T17:08:45+12:00

This PR also removes the "restarts" field which isn't being used and was internal-only. Per #104, we're going to use changes/tasks or events instead.

Shows as follows in the CLI:

$ pebble services
Service  Startup  Current  Start Time
test2    enabled  active   today at 17:08 NZST

$ pebble services --abs-time
Service  Startup  Current  Start Time
test2    enabled  active   2022-04-28T17:08:45+12:00
@benhoyt
Copy link
Contributor Author

benhoyt commented Sep 16, 2022

Per discussion at London mini-sprint, we want to make the API field current-since and display it in the CLI as "Since" (a - if Since is not set). The current-since value would be updated whenever current changed. CLI output example:

$ pebble services
Service  Startup  Current  Since
test2    enabled  active   today at 17:08 NZST

@benhoyt benhoyt changed the title Add "start-time" field to services API and CLI Add "current-since" field to services API and CLI Sep 22, 2022
Copy link
Contributor

@niemeyer niemeyer left a comment

Choose a reason for hiding this comment

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

Very clean PR, thanks Ben.

Name string `json:"name"`
Startup string `json:"startup"`
Current string `json:"current"`
CurrentSince *time.Time `json:"current-since,omitempty"` // pointer as omitempty doesn't work with time.Time directly
Copy link
Contributor

Choose a reason for hiding this comment

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

That's awkward, considering times have IsZero which fits properly with the notion of omitempty.

This is an internal detail, so I'm not blocking on it, but it's curious that the standard json package is misbehaving there.

oldStatus := stateToStatus(s.state)
newStatus := stateToStatus(state)
if oldStatus != newStatus {
s.currentSince = time.Now()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, very clean logic.

@niemeyer niemeyer merged commit e8c856c into canonical:master Dec 5, 2022
@benhoyt benhoyt deleted the services-start-time branch December 5, 2022 20:27
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.

3 participants