netbox-client-ruby icon indicating copy to clipboard operation
netbox-client-ruby copied to clipboard

Dirty-tracking for not-changes attributes?

Open dmke opened this issue 6 years ago • 5 comments

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 avatar May 03 '19 15:05 dmke

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?

TvL2386 avatar Feb 05 '21 15:02 TvL2386

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

thde avatar Feb 12 '21 14:02 thde

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?

TvL2386 avatar Feb 16 '21 09:02 TvL2386

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.

thde avatar Feb 22 '21 08:02 thde

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

TvL2386 avatar Feb 22 '21 10:02 TvL2386