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

Support for "none python" parameters used in the put_object method #108

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

niklasae
Copy link

There are parameters available in the Facebook API that can not be represented by python arguments, for instance "fb:explicitly_shared" (https://developers.facebook.com/docs/opengraph/guides/explicit-sharing/). Therefore there is a need to make the put_object method more flexible. I suggest doing so by allowing a post_args dictionary as a method argument. Since there might be conflicts between the post_args dictionary and the kwargs, I've implemented it in such a way that the kwarg always will take precedence.

@niklasae
Copy link
Author

niklasae commented Jan 8, 2014

Anyone had a look at this yet?

@martey
Copy link
Member

martey commented Jan 8, 2014

I don't have any significant objections to the content of this pull request, and I am glad that you took pains to make it backwards-compatible.

However, I wonder whether Facebook's increasing use of colons in their API methods means that it makes sense to just replace all methods that use kwargs with dictionaries. This would break compatibility in a major way, but would make the code simpler and more future-proof.

@niklasae
Copy link
Author

niklasae commented Jan 8, 2014

That is true and would make sense. The only thing is that I haven't seen it
being used anywhere else, but I'm far from an expert. Does anyone else know
of any other places facebook is using none "dictionary compatible"
arguments?

On 8 January 2014 17:43, Martey Dodoo [email protected] wrote:

I don't have any significant objections to the content of this pull
request, and I am glad that you took pains to make it backwards-compatible.

However, I wonder whether Facebook's increasing use of colons in their API
methods means that it makes sense to just replace all methods that use
kwargs with dictionaries. This would break compatibility in a major way,
but would make the code simpler and more future-proof.


Reply to this email directly or view it on GitHubhttps://github.com//pull/108#issuecomment-31851858
.

@martey
Copy link
Member

martey commented Jan 8, 2014

I think there at least one other place, but it is possible that I am misremembering.

@niklasae
Copy link
Author

niklasae commented Feb 3, 2014

Made any decision regarding this yet? I think it would be great if it was merged, since it doesn't break backwards compatibility and fixes the issue I'm having... 😃

@martey
Copy link
Member

martey commented Feb 3, 2014

I have added this to the 1.0.0 milestone, so I plan on dealing with this before the next release. I have not have a chance to look into whether this commit will be merged directly, or whether larger changes need to be made, though.

@niklasae
Copy link
Author

niklasae commented Feb 4, 2014

Great stuff, let me know if there is anything I can do.

@AlJohri
Copy link

AlJohri commented May 16, 2014

+1 on this PR

@martey martey modified the milestones: v2.0.0, v1.0.0 Jun 5, 2015
@martey martey modified the milestones: v3.0.0, v2.0.0 Aug 8, 2016
@martey martey modified the milestones: v3.0.0, v4.0.0 Jul 19, 2018
@martey martey modified the milestones: v4.0.0, v5.0.0 Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants