Skip to content
This repository has been archived by the owner on Mar 18, 2019. It is now read-only.

URL not constructed in PUT/PATCH methods #137

Open
thinkwelltwd opened this issue Jun 6, 2017 · 12 comments · May be fixed by #138
Open

URL not constructed in PUT/PATCH methods #137

thinkwelltwd opened this issue Jun 6, 2017 · 12 comments · May be fixed by #138

Comments

@thinkwelltwd
Copy link

Thanks for the rest_framework and the core-api library. It's exactly what I need; but I'm pretty sure I've bumped into a core-api bug. Wrote up this stackoverflow post about it.

Would be most grateful to figure out a way through this.

@ghost
Copy link

ghost commented Jun 7, 2017

Hello, I have a strong proof that is related to following line in http transport:

field_map = {field.name: field for field in fields}

This only filters unique keys instead of path + parameter. So if path and parameters are same, it will take one randomly. I had similar behavior between my different models. @tomchristie

@ghost ghost linked a pull request Jun 7, 2017 that will close this issue
@crgwbr
Copy link

crgwbr commented Jul 17, 2017

Any updates on this? I'm being hit by the same issue. Can #138 be merged and released anytime soon?

@thinkwelltwd
Copy link
Author

A polite tap-tap, @tomchristie. Would it be possible to get your eyeballs on this issue, and perhaps merge #138?

@thinkwelltwd thinkwelltwd mentioned this issue Jan 30, 2018
18 tasks
@tomchristie
Copy link
Contributor

Okay, so if anything we should be erroring if there are two parameters with the same name. (rather than forcing the path one to take precedence) Perhaps REST framework's schema generation needs a fix here? Shouldn't cid be read-only in the serializer, and would that fix the issue?

@thinkwelltwd
Copy link
Author

thinkwelltwd commented Jan 30, 2018

Shouldn't cid be read-only in the serializer, and would that fix the issue?

I suppose partly, but when creating a new record, the cid field must be writable. Updating can be read-only. The cid field shouldn't be changed after creation. (Perhaps I should have named the field guid which would've been a more standard abbreviation. cid = Canonical ID)

FWIW, I'm also using AllowPUTAsCreateMixin.

@ryanolf
Copy link

ryanolf commented Mar 28, 2018

There is a related issue when query parameters and form parameters have the same name, which might happen if one has a resource where the query parameters filter on fields of the object. In this case it seems natural to have two parameters with the same name.

One solution for _get_params() might be to loop through the fields and populate values from the params, where available, and then to deal with the parameters that didn't match to any fields. This would allow matching a param to multiple fields. Currently _get_params() loops through params and matches to just one field.

@smn-snkl
Copy link

Any update on this? Running into the same issue as @ryanolf described above. Having a query and form parameter with the same name leads to dropping the query parameter.

@thinkwelltwd
Copy link
Author

thinkwelltwd commented Nov 19, 2018

Per @tomchristie, this project is dead. I wish he'd communicate better about it.

@tomchristie
Copy link
Contributor

tomchristie commented Nov 20, 2018

Personally I'm not making further changes to the project, since my focus is elsewhere.
It's not dead, it's just I'm not putting more work into it from its current state.

Having a query and form parameter with the same name leads to dropping the query parameter.

Probably don't do that.

@thinkwelltwd
Copy link
Author

It's not dead,

Current functionality will continue to work, yes.

it's just I'm not putting more work into it from its current state.

A project where the owner makes no more changes and accepts no more PRs is dead.

Thank you for the project, @tomchristie. The point I'm trying to raise is that it's not at all easy to discern that the project is no longer the focus of your energies and that you've moved to on to focus on other projects. I have no quarrel with that.

I'm only trying to raise awareness to other developers who rely on this project, as I once did, that they shouldn't expect changes.

Again, I would ask that you announce the project's status on the Read Me. Whether you call it deprecated, dead or EOL, I don't care. But let folks know in a prominent location.

@smn-snkl
Copy link

Thanks for bringing this up! And thanks @tomchristie for all the good work, really enjoyed this. Is there a good alternative project you can recommend?

@tomchristie
Copy link
Contributor

API Star is the evolution of this project: https://github.com/encode/apistar

The client library http://docs.apistar.com/client-library/ is OpenAPI driven, which is what's become the standard.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants