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

Video Service issues #3026

Open
MarshalX opened this issue Nov 17, 2024 · 1 comment
Open

Video Service issues #3026

MarshalX opened this issue Nov 17, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@MarshalX
Copy link
Contributor

MarshalX commented Nov 17, 2024

Describe the bug

In Python SDK we have send_video since early days of video support. But this methods utilize uploadBlob method. Which performs all communication with video service under the hood. I really appreciate that you introduced such an easy method to start! But this method has lack of access to processing errors. And it started the common issue for our Python community to ask why "video not found" happens. That's why I started writing the complete example of how to use Video Service directly with job status handling and etc. During the development, I faced with a few really strange things that I want to raise.

I have split this issue to 3 sections. Each represents own problem.

1. Server response is out of the lexicon

To Reproduce

Upload video using uploadVideo endpoint of Video Service

image

You will see the response like this:

image

The response misses jobStatus root object described in the lexicon.

Expected behavior

"output": {
"encoding": "application/json",
"schema": {
"type": "object",
"required": ["jobStatus"],
"properties": {
"jobStatus": {
"type": "ref",
"ref": "app.bsky.video.defs#jobStatus"
}
}
}
}

jobStatus is required field which should be this definition:

"lexicon": 1,
"id": "app.bsky.video.defs",
"defs": {
"jobStatus": {

2. Server status code responses

To Reproduce

Use uploadVideo again with the same video file.

HTTP 409 with Video processed successfully is a bit wild and IMO does not fit XRPC ideology of status codes (https://atproto.com/specs/xrpc#summary-of-http-status-codes)

image

Expected behavior

It is getting really hard to work with. As it requires try-catch for 409 and additional error content deserialization to the proper model. Which is out of the default flow when we parse models in succeed response. It would be really helpful to see the status code from 2xx group.

3. Input of the uploadVideo method is described in the lexicon partially only

To Reproduce

Try to use uploadVideo only with the info described in the lexicon

"input": {
"encoding": "video/mp4"
},

You will be rejected by the server validation that you are missing DID or name input data.

Expected behavior

Probably this could be unique situation when the procedure has both input and parameters, but at least it should be described somehow in the lexicon (IMO). My understanding of missing parameters comes from the traffic sniffing of the official app...

image

Not an issue, just a question

Any participial reason to make name parameter required? As I can see it is randomly generated with the same extension of the uploaded file. Could be easily done on server side? And the DID could be obtained from the service token? Am I wrong?

P.S. I am sorry for being so direct ❣️ I am understand that Video Service is like 3d party service and could do not follow the best practices of atproto, but since is it developed by the same developers I expected more smooth work with it. I just shared my experience. Happy to hear your thoughts. Thank you!

@MarshalX
Copy link
Contributor Author

MarshalX commented Dec 3, 2024

hi @bnewbold could you please take a look or ping the proper people?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant