-
Notifications
You must be signed in to change notification settings - Fork 83
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
cfp: Add Video download url #1768
Conversation
This is used to store a canonical url for the video recording of a CFP item, used to automate release of the recording by means of the schedule frab/json. This is useful in particular when releasing videos via media.ccc.de which can import our frab schedule and automatically download the videos from the listed URLs, but it could also be used to automate youtube uploads, etc. We also add the ability to set this via the proposal api endpoint.
abort(401) | ||
|
||
if auth_header.startswith("bearer "): | ||
bearer_token = auth_header.removeprefix("bearer ") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The contents of the Authorization header is case-sensitive, so "bearer" should be wrong. Can you include a comment to explain why this is needed? (So we can tidy it up when it's no longer the case.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it because I wasted 5 minutes scratching my head why my curl request was giving a 401 ;-)
I went looking for consensus online, always a mistake.. honestly, it seems like a bit of a mess: https://sgeb.io/posts/fix-go-oauth2-case-sensitive-bearer-auth-headers/
Given we're not implementing oauth here I figured it was a convenience and didn't worry about it too much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, fair, # make it easier with curl
would be fine then
Same question about the contents of the authorization header, but apart from that: looks good to me 👍 |
I don't know whether this would be necessary -- it depends on the workflows and server URL schemes involved -- but we could perhaps assert that the stored values don't contain any authentication details ( |
Is the schedule format adjusted here only presented through the |
From IRC:
Currently it will - since AFAICT the static export just dumps the response of the frab/json endpoints to disk. So we probably need to filter that out. Part of me wonders if this is another reason to have a url param to include video_download_url in the schedule responses... |
@jayaddison
|
Closing - will generate this separately since it's a bit single-purpose and doesn't really need to live in the website. |
This is used to store a canonical url for the video recording of a CFP item, used to automate release of the recording by means of the schedule frab/json.
This is useful in particular when releasing videos via media.ccc.de which can import our frab schedule and automatically download the videos from the listed URLs, but it could also be used to automate youtube uploads, etc.
We also add the ability to set this via the proposal api endpoint.
I'm not entirely convinced this should this be implemented in this way though: