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

feat: show query start time in progress spinner #798

Merged
merged 2 commits into from
Jun 1, 2022

Conversation

dmurray-lacework
Copy link
Collaborator

@dmurray-lacework dmurray-lacework commented May 25, 2022

Signed-off-by: Darren Murray [email protected]

Summary

When running the lacework query run command, display the start time and end time of time range the query is executing in the progress spinner.

How did you test this change?

run cmd
lacework query run <query-id>
lacework query run <query-id> --start -1hr

example output:

>  Executing query in the time range 2022-May-25 13:12:43 UTC - 2022-May-26 13:12:43 UTC 

Issue

https://lacework.atlassian.net/browse/ALLY-938

cli/cmd/lql.go Show resolved Hide resolved
cli/cmd/lql.go Show resolved Hide resolved
Copy link
Contributor

@afiune afiune left a comment

Choose a reason for hiding this comment

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

tenor-88378181

cli.StartProgress(" Executing query...")
msg := "Executing query"
startTime, startErr := time.Parse(time.RFC3339, args[0].Value)
endTime, endErr := time.Parse(time.RFC3339, args[1].Value)
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel the way this function is written allows us to make easy mistakes such as 1) having the arguments in a different order and getting the start/end time wrong, and 2) passing less than two arguments which will throw a panic trying to access elements that don't exist since we are not checking that the array has at least two elements.

There are no unit tests for this function.

@hazedav @dmurray-lacework let's improve this code so we don't leave room for human mistakes. It can be done with further PRs, but let us not forget about this. Please.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cli.StartProgress(" Executing query...")
msg := "Executing query"
startTime, startErr := time.Parse(time.RFC3339, args[0].Value)
endTime, endErr := time.Parse(time.RFC3339, args[1].Value)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for this function runAdhocQuery()

@dmurray-lacework dmurray-lacework merged commit 2179616 into main Jun 1, 2022
@dmurray-lacework dmurray-lacework deleted the dmurray-lacework/ALLY-938 branch June 1, 2022 13:53
@lacework-releng lacework-releng mentioned this pull request Jun 7, 2022
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