ninech / netbox-client-ruby

A ruby client library for Netbox v2.
MIT License
24 stars 21 forks source link

Dirty-tracking for not-changes attributes? #31

Open dmke opened 5 years ago

dmke commented 5 years ago

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]
TvL2386 commented 3 years ago

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 commented 3 years ago

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

TvL2386 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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