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: add stream endpoint #504

Merged
merged 5 commits into from
Aug 16, 2022
Merged

feat: add stream endpoint #504

merged 5 commits into from
Aug 16, 2022

Conversation

bwanglzu
Copy link
Member

@bwanglzu bwanglzu commented Aug 12, 2022

Finetuner-API now supports a new endpoint: /logstream, in this PR we allow the client to subscribe this endpoint.

import finetuner

finetuner.login()

run = finetuner.get_run(
    run_name='log-streaming-exp-run-7',
    experiment_name='log-streaming-exp'
)

for log_entry in run.stream_logs():
    print(log_entry)

image


  • This PR references an open issue
  • I have added a line about this change to CHANGELOG

@bwanglzu bwanglzu marked this pull request as ready for review August 15, 2022 09:31
Copy link
Member

@jupyterjazz jupyterjazz left a comment

Choose a reason for hiding this comment

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

Looks good!

Just one comment:
It looks like this is the only intended way of getting the logs:

for log_entry in run.stream_logs():
    print(log_entry)

so why not combining it into run.logstream() or something?

@bwanglzu
Copy link
Member Author

@jupyterjazz we'll discuss tomorrow when @gmastrapas is back

Copy link
Member

@gmastrapas gmastrapas left a comment

Choose a reason for hiding this comment

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

LGTM!

@bwanglzu bwanglzu merged commit 7af6234 into main Aug 16, 2022
@bwanglzu bwanglzu deleted the feat-log-stream branch August 16, 2022 15:27
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.

None yet

3 participants