search

tus / tus-resumable-upload-protocol

Open Protocol for Resumable File Uploads
https://tus.io
MIT License
1.48k stars 103 forks source link

Initial OpenAPI specification of the tus protocol. #170

Closed RubenGarcia closed 3 years ago

RubenGarcia commented 3 years ago

Following https://github.com/tus/tus-resumable-upload-protocol/issues/169

RubenGarcia commented 3 years ago

Is 460 Checksums mismatch used somewhere else outside of tus?

RubenGarcia commented 3 years ago

This part is missing:

The Tus-Resumable header MUST be included in every request and response except for OPTIONS requests. The value MUST be the version of the protocol used by the Client or the Server.

If the version specified by the Client is not supported by the Server, it MUST respond with the 412 Precondition Failed status and MUST include the Tus-Version header into the response. In addition, the Server MUST NOT process the request.

Acconut commented 3 years ago

Thank you for your incredible work here! Could you help me, as person who hasn't used OpenAPI before, with some basic questions:

  1. What is the difference between OpenAPI 2 and 3? Do we need both version or could we just use the newer one?
  2. How can people use these files? What tools does it integrate with?
  3. I see redundant definition of some header (e.g. Tus-Resumable and Upload-Offset). Would it be possible to deduplicate them?
RubenGarcia commented 3 years ago
  1. OpenAPI 3 is the newer, more capable standard. But many tools still use OpenAPI 2, so it's worth keeping both. There are tools and websites to convert and help in creating the files, such as swagger.io.
  2. Once you have those files, there are tools to create http servers and clients to access the API; missing functionality (semantics) is then to be added manually to the relevant code snippets. e.g. go-swagger, or the generators available at swagger.io.
  3. Yes, in OpenAPI 3. No in OpenAPI 2.0 https://swagger.io/docs/specification/2-0/describing-responses/. Once the OpenAPI 2.0 is complete and is in a state where merge is possible, I will convert to 3.0 and optimize.
RubenGarcia commented 3 years ago

Do you see anything I may have missed from the standard in the OpenAPI 2.0 file?

Acconut commented 3 years ago

There are tools and websites to convert

Would it then make sense to only keep the OpenAPI 3 version in this repository and let the users convert it to OpenAPI 2 when necessary? I am not really a fan of having two OpenAPI files with basically the same content in this repository. Maintaining two files which are similar but also have differences is quite erroneous, especially since most contributors here are not OpenAPI experts (including me). That would also solve the problem with the duplicate header definitions.

smatsson commented 3 years ago

@RubenGarcia Your work here is truly amazing. Nice done!

RubenGarcia commented 3 years ago

I just checked https://editor.swagger.io. You can convert from 2.0 to 3.0 but not from 3.0 to 2.0, I think because 3.0 added more functionality. And since the 3.0 would needs to be modified to merge the duplicate definitions, I think there is value in keeping both. I don't think the effort of keeping both is too high, because the protocol is quite stable now.

Acconut commented 3 years ago

From a quick Google search, I found https://lucybot-inc.github.io/api-spec-converter/ and https://github.com/LucyBot-Inc/api-spec-converter for converting from 3.0 to 2.0. Of course, I didn't try them yet. Do you have experience with them and can judge if they are helpful or not?

because the protocol is quite stable now.

Yes, the protocol is stable but we have some new extensions lined up. So there are likely going to be some additions in the next weeks.

RubenGarcia commented 3 years ago

I was not aware of these. Ok, then I'll make a 3.01 and add a note to use these as needed if you need 2.0.

RubenGarcia commented 3 years ago

I moved the definitions of the headers which are used in various places to the end to avoid repetition. I kept the two separate instances of Content-Length because they have different semantics (i.e. one of them must be 0 if no further extensions are available). I kept only OpenAPI 3.0 and added a comment in the readme about conversion software.

Acconut commented 3 years ago

Thank you very much, Ruben! I will have a more in depth look at the content next week. Do you know if a good validator/linter exists, so we can ensure that we don't commit invalid OpenAPI specifications?

RubenGarcia commented 3 years ago

I am using the swagger editor on swagger.io

Acconut commented 3 years ago

I am using the swagger editor on swagger.io

Ok, I was more thinking about a CLI tool or similar that we could automatically run for every commit to ensure that the OpenAPI file is not malformed. Do you know if that exists?

RubenGarcia commented 3 years ago

Other tools:

Or this one

Would you like me to setup super-linter in this repository?

Acconut commented 3 years ago

Hello @RubenGarcia, I want to apologize for my late reply! I do not know how but I lost this discussion totally from my radar! I am deeply sorry for that after all of the efforts you invested into this!

super-linter in combination with Spectral looks like the tools we had in mind! Are you still interested in moving forward with this topic?

RubenGarcia commented 3 years ago

Yes. I don't have experience with superlinter, but I think it's worth exploring. Do I need administrator priviledges or anything special?

RubenGarcia commented 3 years ago

I added the configuration using https://github.com/github/super-linter and https://github.com/marketplace/actions/spectral-linting but I think CICD needs to be setup on your side before it will run.

Acconut commented 3 years ago

Thank you very, very much! I will merge this, so we can see if the GH Action works. We can iterate over the specification in further PRs.

Acconut commented 3 years ago

Seems like there is still a configuration problem with bad GitHub credentials: https://github.com/tus/tus-resumable-upload-protocol/runs/1782659818?check_suite_focus=true#step:4:8

RubenGarcia commented 3 years ago

Seems like there is still a configuration problem with bad GitHub credentials: https://github.com/tus/tus-resumable-upload-protocol/runs/1782659818?check_suite_focus=true#step:4:8

I just checked that link, and it shows green (success) for me, so I guess all is ok now.