Skip to content

Allow step to be None #98

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Allow step to be None #98

wants to merge 4 commits into from

Conversation

kgodlewski
Copy link
Contributor

@kgodlewski kgodlewski commented Dec 2, 2024 •
edited
Loading

Changes:

  • change typehints for step to Optional
  • send empty step to the backend, as it will auto-increment it for a given metric
  • don't merge RunOperations when step is None, as it would overwrite subsequent log() calls with None steps
  • perform preliminary client-side validation of the most recent step-value pair for a given metric, to catch errors early on

@kgodlewski kgodlewski changed the base branch from main to kg/refactor-attribute-store December 2, 2024 15:41
@kgodlewski kgodlewski force-pushed the kg/auto-step branch 2 times, most recently from a936b3f to f1d68e9 Compare December 4, 2024 12:03
@kgodlewski kgodlewski force-pushed the kg/refactor-attribute-store branch from 0c26195 to 70fa6be Compare December 6, 2024 13:16
@PatrykGala PatrykGala marked this pull request as draft December 9, 2024 13:21
@kgodlewski
Allow step to be None.
1856e2e 
Validate if steps are increasing client-side for early error detection.
@kgodlewski
Assume float timestamps are UTC.
89c5302 
`time.time()` returns unix timestamp in utc, so this seems to be a
reasonable assumption.
@kgodlewski
Make step in log_metrics optional.
3493196 
Passing `None` as `step` now automatically increments the step for a
given metric.
@kgodlewski
Copy link
Contributor Author

This feature is pushed forward on the roadmap, so closing it for now. A new PR that contains only local step validation is open instead: #104

@kgodlewski kgodlewski closed this Dec 17, 2024
@kgodlewski kgodlewski deleted the kg/auto-step branch December 18, 2024 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michalsosn michalsosn Awaiting requested review from michalsosn

@PatrykGala PatrykGala Awaiting requested review from PatrykGala

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@kgodlewski