-
Notifications
You must be signed in to change notification settings - Fork 209
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
Small issues that I ran into running tests #198
Comments
okay, done with testing! https://vsoch.github.io/django-oci/conformance/ This issue is ready to be looked at. There are still some tweaks and additional tests I want to do with django-oci, and then after come up with some nice documentation and more storage backends, but I think the hard bit (the specification adherence) is almost there! 🎉 |
@vsoch - very cool to see a registry implemented from spec! Here are some answers:
I'm confused what you mean. What do you mean text-plain is fine? As far as conformance, your registry should at least allow octet-stream, and up to you for how strict you want to be.
I hadn't come across this issue specifically, but perhaps its worth stating somewhere in FAQ the list of content types that must be allowed
What you are referring to here is actually a type of monolithic upload. In the spec:
This is number 2. Perhaps is can be better stated in the test suite?
Yes, this is under review here: #171
Would this not be considered an out-of-order upload? We could add a test for this too.
Do you have an example? I cant think of an instance of a URL that would/should have a trailing slash, besides /uploads/ session URL.
Would you consider this a separate issue from this discussion? #180
I hope you have recovered from this piano incident :) But I do think it's evident that the requests to this API should be as dynamic as possible to reflect whats in the registry. I think some of your issues are a function of using a framework (Django) designed for browser apps vs. APIs |
I mean to say that I was expecting a content type of application/octet-stream (e.g., for my registry I was validating it and returning an error to get a different content type) but for the conformance test it was sending along text/plain so "text plain is fine." In practice I would have expected the content-type header to have been added here.
I think we are talking about different things, because I'm not referencing a POST but rather a PATCH. I think I read that the PATCH should have the content range. I think the two requests for uploads with POST start on lines 63 and 85.
Definitely, and maybe a test would help because it feels like an edge case.
I think it would be enough to state it explicitly - a lot of frameworks will automatically add the slash, so it's more for the developer to know to disable/enable that.
Yep I can watch this issue, thanks!
The Piano Doctor fixed me up :_) You're correct that Django is a general MVC framework, but it makes it relatively easy to design an API and then of course models in the database and views to match. It's really nice for creating a plugin (the models, urls, and views) for someone else to easily plugin into their application, which is why I chose it. |
I am currently working through conformance testing for my plugin, and I want to open an issue to put notes / things to discuss. Think of this as in progress - I'll likely need a little bit to get through all the tests. I will clearly update the issue (at the end) to indicate completion when it's done. Feel free to disregard until then :)
Conformance testing notes
I ran into some small discrepancies, or more like points that weren't clearly stated in the spec that might be worth making more transparent.
The text was updated successfully, but these errors were encountered: