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

Add Payload property in the Video Model #2

Closed
wants to merge 1 commit into from

Conversation

tgallice
Copy link
Contributor

@tgallice tgallice commented Jul 8, 2014

Actually the Video Model don't have the payloadproperty. The issue is when we want to transform a Json response with the payload attribute to a Video object. An error is raise by the Normalizer who doesn't find the setter

xabbuh added a commit that referenced this pull request Jul 9, 2014
xabbuh added a commit that referenced this pull request Jul 9, 2014
@xabbuh
Copy link
Owner

xabbuh commented Jul 9, 2014

Thanks! I merged this in at 303b163.

@xabbuh xabbuh closed this Jul 9, 2014
@xabbuh
Copy link
Owner

xabbuh commented Jul 9, 2014

Actually, I'm not really sure anymore if it is a good idea to fail when hitting unknown property. Instead those could be silently ignored. What do think?

@tgallice tgallice deleted the add_payload branch July 10, 2014 08:00
@stof
Copy link
Contributor

stof commented Jul 10, 2014

@xabbuh ignoring unknown properties would indeed be safer. Adding additional properties in an API response is generally not considered as a BC breaking change, so clients should accept them even if they are not able to use them.

xabbuh added a commit that referenced this pull request Jul 10, 2014
These changes are namely (in chronological order):

* added a payload property to the ``Video`` model (by @tgallice in #2)

* additional options (a list of profiles to be used, a custom path format
  a payload) can be passed to the methods ``CloudInterface::encodeVideoByUrl()``
  and ``ClientInterface::encodeVideoFile()``
xabbuh added a commit that referenced this pull request Jul 10, 2014
As pointed out in #2, additional properties that are not defined in
the model classes lead to exceptions thrown by the normalizer. Since,
those additional properties are no backward compatibility breaks in
an API, they could be added on the remote site without any notification
and must not break the client.
xabbuh added a commit that referenced this pull request Jul 10, 2014
As pointed out in #2, additional properties that are not defined in
the model classes lead to exceptions thrown by the normalizer. Since,
those additional properties are no backward compatibility breaks in
an API, they could be added on the remote site without any notification
and must not break the client.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants