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

WIP: Upload build logs to S3 #1161

Closed
wants to merge 4 commits into from
Closed

Conversation

manics
Copy link
Member

@manics manics commented Oct 4, 2020

Playing around with S3 logs again (#1156). This time uploading repo2docker logs from the BinderHub side instead of the repo2docker side.

aioboto* requires compiled dependencies which may not be available as wheels
This ensures logs are uploaded for failed builds
@minrk
Copy link
Member

minrk commented Oct 7, 2020

Following comments in jupyterhub/repo2docker#967 , if we do it on the binderhub side, I think a sidecar container in the build pod might be a better way to go because we don't have to think about the consequences of binder pod replicas and restarts. A build can have 0, 1, or 2 (or more) RequestHandlers and binder pods watching it, whereas doing it in repo2docker (or in the build pod at least) should mean the lifecycle and everything is more naturally aligned.

Cases we would need to handle if we did at the level in this PR:

  • 0 watchers if no requests are currently connected
  • 2 (or more) watchers if multiple requests to watch a build of the same image are routed to different binder replicas. There's still one build pod, but multiple pods watching the build logs. This may even happen with multiple requests to one pod, I'd need to refresh my memory, though.

@manics
Copy link
Member Author

manics commented Oct 7, 2020

Thanks... after reading the feedback on jupyterhub/repo2docker#967 I think r2d is the best (and easiest) place for it, let's continue on jupyterhub/repo2docker#967

@manics manics closed this Oct 7, 2020
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