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(performance-metrics): store to file #14882

Merged
merged 5 commits into from
Apr 12, 2024

Conversation

DerekMaggio
Copy link
Contributor

Overview

Add functionality to store CSV data in a file

Test Plan

  • Added test cases to verify storing to a file correctly

Changelog

  • Added store method to RobotContextTracker
  • Added data shaping to RawContextData
  • Tests

Review requests

None

Risk assessment

Low

@DerekMaggio DerekMaggio requested review from a team as code owners April 11, 2024 20:52
@DerekMaggio DerekMaggio changed the base branch from edge to EXEC-396-add-test-lint-github-action April 11, 2024 20:53
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

LGTM!

"""Returns the stored context data and clears the storage list."""
stored_data = self._storage.copy()
self._storage.clear()
rows_to_write = [context_data.csv_row() for context_data in stored_data]
Copy link
Member

Choose a reason for hiding this comment

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

we should probably sort this by start time before writing just in case - once we add various queues or whatever reordering might become possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Won't it always be sortable by start time regardless? My thought is ONLY robot context data would go in this file. Other metrics like CPU/Memory/Flash would be in a separate file. That way all this does is just push data to a file as fast as possible.

Then when those files are pulled for analysis they can be sorted at that time.

add workflow dispatch so I can test it

chore: setup python correctly

didn't add the right thing to test it

why won't you run

just run

oh. my. gosh. run!

argh

sgdf

i know I am doing something dumb

sigh

audible sigh

yeah, I was doing something stupid

well that was an adventure
@DerekMaggio DerekMaggio force-pushed the EXEC-396-add-test-lint-github-action branch from d42ab38 to d75493a Compare April 12, 2024 14:50
Base automatically changed from EXEC-396-add-test-lint-github-action to EXEC-384-robot-context-tracker April 12, 2024 14:52
@DerekMaggio DerekMaggio force-pushed the EXEC-377-store-to-file branch from 66601f0 to abc128a Compare April 12, 2024 14:55
@DerekMaggio DerekMaggio merged commit 40092d9 into EXEC-384-robot-context-tracker Apr 12, 2024
5 checks passed
@DerekMaggio DerekMaggio deleted the EXEC-377-store-to-file branch April 12, 2024 14:59
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.

2 participants