Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Only send deltas #69

Open
wants to merge 3 commits into
from

Conversation

Projects
None yet
7 participants
Contributor

fictorial commented Feb 6, 2013

I am by no-means a "Ruby guy" (I am visiting from Pythonland) but this works for me.

This is for issue # 68

#68

I think BOTH of these lines need to be included actually.

Owner

adelevie commented Feb 6, 2013

Awesome, thanks!

Python is great. Welcome.

I'll look at the code in the morning, but can you undo the version bump?

Sent from my iPhone

On Feb 5, 2013, at 11:52 PM, Brian Hammond notifications@github.com wrote:

I am by no-means a "Ruby guy" (I am visiting from Pythonland) but this works for me.

This is for issue # 68

#68

You can merge this Pull Request by running

git pull https://github.com/fictorial/parse-ruby-client master
Or view, comment on, or merge it at:

#69

Commit Summary

Only send deltas on save
Bump version to 0.1.13
File Changes

M VERSION (2)
M lib/parse/object.rb (19)
M parse-ruby-client.gemspec (2)
Patch Links:

https://github.com/adelevie/parse-ruby-client/pull/69.patch
https://github.com/adelevie/parse-ruby-client/pull/69.diff

Contributor

fictorial commented Feb 6, 2013

Hi, have you had a chance to look at this request? I need this for a client project and am not sure how to include my own copy of a gem locally with a ruby script. Otherwise I'd be quiet and sit tight. :-) Thanks

Owner

adelevie commented Feb 7, 2013

Try using this in your Gemfile:

gem "parse-ruby-client", :git => "git://github.com/fictorial/parse-ruby-client.git"

Notice that it points to your fork on GitHub.

Contributor

fictorial commented Feb 7, 2013

Awesome! Thanks.

Contributor

gmaliar commented Feb 10, 2013

Or you could put the fixed gem inside your lib/ directory in your rails application and do
ruby gem "parse-ruby-client", :path => "lib/path/to/your/gem/root"

@ericcj ericcj commented on the diff Feb 11, 2013

lib/parse/object.rb
@@ -213,6 +223,11 @@ def decrement(field, amount = 1)
self
end
+ def []=(key, value)
@ericcj

ericcj Feb 11, 2013

Contributor

need to override all the mutation paths on Hash not just assignment, e.g. #merge!

@adelevie

adelevie Feb 11, 2013

Owner

Maybe use some kind of dirty tracker? Only PUT some value if its key is dirty?

Sent from my iPhone

On Feb 11, 2013, at 9:11 AM, Eric Jensen notifications@github.com wrote:

In lib/parse/object.rb:

@@ -213,6 +223,11 @@ def decrement(field, amount = 1)
self
end

  • def []=(key, value)
    need to override all the mutation paths on Hash not just assignment, e.g. #merge!


Reply to this email directly or view it on GitHub..

@adelevie

adelevie Feb 11, 2013

Owner

Please tell me there's a language feature in ruby for this. If not, I'm thinking of passing on this one. It would be way too messy.

Sent from my iPhone

On Feb 11, 2013, at 9:11 AM, Eric Jensen notifications@github.com wrote:

In lib/parse/object.rb:

@@ -213,6 +223,11 @@ def decrement(field, amount = 1)
self
end

  • def []=(key, value)
    need to override all the mutation paths on Hash not just assignment, e.g. #merge!


Reply to this email directly or view it on GitHub..

@ericcj ericcj commented on the diff Feb 11, 2013

lib/parse/object.rb
@@ -69,7 +72,14 @@ def new?
end
def safe_hash
- without_reserved = self.dup
+ without_reserved = nil
+
+ if @changes.length == 0 then
@ericcj

ericcj Feb 11, 2013

Contributor

deciding which hash to use (changes or self) based on whether changes are present introduces a third way of deciding if we're updating or creating. one effect is that this object would be half-saved:

obj = Parse::Object.new('Foo')
obj.merge!('bar' => 1)
obj['baz']
obj.save

we should centralize these:

  1. #save uses "if @parse_object_id"
  2. #new? uses self["objectId"].nil?
  3. this uses @changes.empty?

it never makes sense to be POST'ing a subset of fields on creation. it would probably be safer to filter the PUT to the fields that are changed than to maintain a parallel hash of their actual values.

Contributor

ericcj commented Feb 11, 2013

the big challenge here is what to do about mutations of values that are not assignments e.g. adding to an existing array field, updating a mutable value (either a native ruby type or one of the parse datatypes like File or GeoPoint). it goes hand in hand with the question of whether Parse::Object should directly extend Hash. we should probably handle it by a more significant refactor and include significant testing and a major version bump.

Owner

adelevie commented Apr 8, 2013

I'm going to close this and not accept. If anyone gives me a reason to reopen/accept, I'm all ears.

@adelevie adelevie closed this Apr 8, 2013

Contributor

fictorial commented Apr 8, 2013

Well, the clear reason is to not send a potentially very large amount of data when on a very small amount of data would suffice.

Owner

adelevie commented Apr 8, 2013

@fictorial I get the reasoning behind this, and this would be a tremendously good feature to have, but the PR doesn't provide a very workable solution. See #69 (comment)

@adelevie adelevie reopened this Apr 8, 2013

Owner

adelevie commented Apr 8, 2013

#67 shows a good reason why this problem should be solved.

Contributor

gmaliar commented Apr 10, 2013

Perhaps Parse::Object can have some meta programming on it to create the dirty trackers when objects are parsed from Parse's servers or when the user creates a new objcet but as @ericcj noted there needs to be some extra work on mutables and I don't think it can extend the Hash object.

mstorch commented Sep 1, 2015

Are there any current plans to re-address this pull request? Or are there any other solutions in the works to track dirty fields and only send those during a save?

This would be a very helpful feature, let me know if any help is needed to get this integrated. Thanks!

Collaborator

rhymes commented Sep 3, 2015

@mstorch I feel like this feature has to be rewritten completely. Anyway the PR has no tests so we can't merge this. Do you feel like writing this feature from scratch?

mstorch commented Sep 3, 2015

I probably don't have enough cycles at the moment to write a solution from scratch that includes a full suite of tests. But if some of my time frees up, I would be willing. I will stay tuned in case anyone picks this up and in that case I will help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment