Apipie / apypie

Apipie bindings for Python
https://apypie.readthedocs.io/en/latest/
MIT License
1 stars 9 forks source link

Encode query_string params as string #78

Closed mdellweg closed 4 years ago

mdellweg commented 4 years ago
evgeni commented 4 years ago

I guess this breaks FAM tests as the fixtures contain uppercase True in queries for "thin"?

evgeni commented 4 years ago

Also, how does that influence, if at all, unicode params (on Python 2)? Probably safer to avoid the non-boolean str cast and let requests decide what to do?

mdellweg commented 4 years ago

Also, how does that influence, if at all, unicode params (on Python 2)? Probably safer to avoid the non-boolean str cast and let requests decide what to do?

As in "not change the part that is not broken"? I'll do that. But the fam fixtures will still be wrong... I sure hope that the other parts of the api accept "true" where we sent "True" before.

evgeni commented 4 years ago

My best guess is: it didn't, but also didn't complain. 🤷‍♀️

evgeni commented 4 years ago

https://github.com/theforeman/foreman/blob/d00fc37a68839f8bf823969880339c9343eacd4c/app/controllers/api/v2/hosts_controller.rb#L44 looks like "any string that's not 'false' will be true"?

evgeni commented 4 years ago

Yeah...

https://github.com/theforeman/foreman-ansible-modules/blob/fe0f1d0d6f1ccd5cf2d00ff0794c7f5b940a6a32/tests/test_playbooks/fixtures/host-2.yml#L86

There is thin=False but the response in the next lines only contains id and name…

mdellweg commented 4 years ago

I which order are we supposed to fix/merge this?

evgeni commented 4 years ago

I think we can just accept the failing test here for now, merge it, release a new version and then re-record/sed the fixtures.

Also having #73 in would be good for the release (and the vendoring ;))

evgeni commented 4 years ago

@mdellweg would you mind rebasing this once, so it runs with pylint, just to be sure? :)

evgeni commented 4 years ago

Ah, needs rebase anyways whistle

mdellweg commented 4 years ago

pylint seems happy.