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

control-service: fix log link to set endTime always #735

Merged
merged 2 commits into from
Feb 23, 2022

Conversation

antoniivanov
Copy link
Collaborator

In case the job is not finished, there's no end time (logically) but
VMware Log Insight links do not work if no endTime is set. That might be
problem for other engines.

It's better to set it to current time instead of leaving empty.

I was wondering if I should set it to 1 hour in the future. But uses can
use endOffset option to change this. And there's chance that some Log
services may not work well with "future" times.

Testing Done: unit tests

Signed-off-by: Antoni Ivanov [email protected]

In case the job is not finished, there's no end time (logically) but
VMware Log Insight links do not work if no endTime is set. That might be
problem for other engines.

It's better to set it to current time instead of leaving empty.

I was wondering if I should set it to 1 hour in the future. But uses can
use endOffset option to change this. And there's chance that some Log
services may not work well with "future" times.

Testing Done: unit tests

Signed-off-by: Antoni Ivanov <[email protected]>
@antoniivanov antoniivanov force-pushed the person/aivanov/control-service-time branch from 4910ef4 to 1f82c59 Compare February 23, 2022 15:17
@antoniivanov antoniivanov enabled auto-merge (squash) February 23, 2022 15:41
@antoniivanov antoniivanov merged commit 4223b27 into main Feb 23, 2022
@antoniivanov antoniivanov deleted the person/aivanov/control-service-time branch February 23, 2022 15:41
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.

3 participants