Skip to content

api: add timestamp to task status message#320

Merged
aaronlehmann merged 1 commit into
moby:masterfrom
stevvooe:taskstatus-timestamp
Apr 14, 2016
Merged

api: add timestamp to task status message#320
aaronlehmann merged 1 commit into
moby:masterfrom
stevvooe:taskstatus-timestamp

Conversation

@stevvooe
Copy link
Copy Markdown
Contributor

No description provided.

@stevvooe stevvooe force-pushed the taskstatus-timestamp branch 3 times, most recently from 9a9a5ad to be31f27 Compare April 14, 2016 06:57
@docker-codecov-bot
Copy link
Copy Markdown

Current coverage is 52.00%

Merging #320 into master will increase coverage by +0.09% as of 8776311

@@            master   #320   diff @@
=====================================
  Files           49     50     +1
  Stmts         7239   7274    +35
  Branches       963    968     +5
  Methods          0      0       
=====================================
+ Hit           3758   3783    +25
- Partial        544    545     +1
- Missed        2937   2946     +9

Review entire Coverage Diff as of 8776311


Uncovered Suggestions

  1. +0.58% via ...ager/state/memory.go#293...334
  2. +0.32% via agent/session.go#193...215
  3. +0.32% via ...y/test/service.pb.go#687...709
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

@aluzzardi
Copy link
Copy Markdown
Member

LGTM - needs rebase

@stevvooe stevvooe force-pushed the taskstatus-timestamp branch from be31f27 to 8776311 Compare April 14, 2016 21:19
Comment thread agent/agent.go Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Where does this get set?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

status has the type *api.TaskStatus, not taskStatusReport.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Adds the timestamp type to TaskStatus. This can open up some interesting
behavior in the scheduler. For now, we let the agent set it, which may
not be ideal.

Note that we've had to do some nasty protobuf wrangling. External types
still aren't well supported. We'll be able to remove all of this once
gogoprotobuf supports timestamp.

Signed-off-by: Stephen J Day <stephen.day@docker.com>
@stevvooe stevvooe force-pushed the taskstatus-timestamp branch from 8776311 to 8d93e6a Compare April 14, 2016 22:50
@aaronlehmann
Copy link
Copy Markdown
Collaborator

LGTM

@aaronlehmann aaronlehmann merged commit 651e5f7 into moby:master Apr 14, 2016
@aaronlehmann aaronlehmann deleted the taskstatus-timestamp branch April 14, 2016 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants