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

Dirty-tracking for not-changes attributes? #31

Open
dmke opened this issue May 3, 2019 · 5 comments
Open

Dirty-tracking for not-changes attributes? #31

dmke opened this issue May 3, 2019 · 5 comments

Comments

@dmke
Copy link

dmke commented May 3, 2019

I'm currently importing a larger number of configurations from different sources into Netbox, and this causes a lot of (unnecessary) PATCH requests.

Let's say I've got this device role, created by a previous import run:

role = NetboxClientRuby.dcim.device_roles.find_by(slug: "access-point")
#=> #<NetboxClientRuby::DCIM::DeviceRole @data={"id"=>6, "name"=>"Access Point", "slug"=>"access-point", "color"=>"009688", "vm_role"=>false}, @dirty_data={}, @id=6>

When setting its attributes, it gets tracked:

role.name = role.name
role.send :dirty_data
#=> {"name"=>"Access Point"}

Should NetboxClientRuby::Entity#method_missing track dirty attributes when the value did not change?

The patch should be relatively easy:

@@ lib/netbox_client_ruby/entity.rb @@
     def method_missing(name_as_symbol, *args, &block)
       if name.end_with?('=')
          ...
+        dirty_data[name[0..-2]] = args[0] unless dirty_data[name[0..-2]] == args[0] 
-        dirty_data[name[0..-2]] = args[0]
         return arg[0]
@dmke dmke changed the title Dirty-tracking for non-dirty attributes? Dirty-tracking for not-changes attributes? May 3, 2019
@TvL2386
Copy link
Contributor

TvL2386 commented Feb 5, 2021

This would be a great feature, since I am syncing data from a management tool to netbox.
For devices in my tool, I find or initialize DCIM::Interface objects.
It would be great if I can present a save button on #is_dirty? or #changed?

@thde
Copy link
Member

thde commented Feb 12, 2021

I'm happy to review and merge if you find time for a PR.

@TvL2386
Copy link
Contributor

TvL2386 commented Feb 16, 2021

I added the code which I thought would be best to implement this feature, however it breaks the test suite and I don't know why exactly. Maybe some of the tests are actually using the fact that dirty_data is being set when the object is actually not changed?

diff --git a/lib/netbox_client_ruby/entity.rb b/lib/netbox_client_ruby/entity.rb
index 726fab4..4cf7b4b 100644
--- a/lib/netbox_client_ruby/entity.rb
+++ b/lib/netbox_client_ruby/entity.rb
@@ -135,6 +135,10 @@ module NetboxClientRuby
       self
     end
 
+    def changed?
+      !dirty_data.empty?
+    end
+
     def save
       return post unless ids_set?
       patch
@@ -200,6 +204,11 @@ module NetboxClientRuby
 
         return super if not_this_classes_business
 
+        if ids_set? and data[name[0..-2]] == args[0]
+          dirty_data.delete name[0..-2]
+          return args[0]
+        end
+
         dirty_data[name[0..-2]] = args[0]
         return args[0]
       end

In the bin/console this works great:

[1] pry(main)> int = NetboxClientRuby.dcim.interfaces.first
=> #<NetboxClientRuby::DCIM::Interface:0x00005632a1fc2ba8
[2] pry(main)> int.changed?
=> false
[3] pry(main)> int.name = 'something'
=> "something"
[4] pry(main)> int.changed?
=> true
[5] pry(main)> int.name = 'interfaces 0'
=> "interfaces 0"
[6] pry(main)> int.changed?
=> false

Do you have any suggestions?

@thde
Copy link
Member

thde commented Feb 22, 2021

I don't really have time to look into it too deeply ATM. But if you make an MR, I can have a look at the failing specs.

@TvL2386
Copy link
Contributor

TvL2386 commented Feb 22, 2021

Hi thde,
I created merge request #43
Kind regards!

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

No branches or pull requests

3 participants