bjjb / ebayr

A small library to help using the eBay Trading API with Ruby
MIT License
60 stars 49 forks source link

Allow more than one instance of the Ebay API connection #22

Open meismann opened 8 years ago

meismann commented 8 years ago

Use cases:

The new usage pattern may then look like this:

ebay1 = Ebayr::API.new(configuration_hash_from_file)
ebay.call(:GetSuggestedCategories, options_1)

ebay2 = Ebayr::API.new(another_configuration_hash_from_file)
ebay.call(:GetItem, options_2)

allthewhile maintaining backward compatibility…

meismann commented 8 years ago

I am going to implement this feature now and use it for my client. I will consider your comments/suggestions before making a PR.

meismann commented 8 years ago

Just found out that this works:

2.2.2 :014 > class BB; include Ebayr.dup;end
 => BB 
2.2.2 :015 > class AA; include Ebayr.dup;end
 => AA 
2.2.2 :016 > bb = BB.new
 => #<BB:0x007fae9d195890> 
2.2.2 :017 > bb.ru_name = 1
 => 1 
2.2.2 :018 > aa = AA.new
 => #<AA:0x007fae9c9da178> 
2.2.2 :020 > aa.ru_name = 3
 => 3 
2.2.2 :021 > aa.ru_name 
 => 3 
2.2.2 :022 > bb.ru_name
 => 1 

So, instances of AA and BB can be used for different connections. Not sure if I like this solution. I am bothered by the dup in the include.

UPDATE: This wont work either, since Ebayr::Request includes Ebayr and not the dupped version we are using.

bjjb commented 8 years ago

@meismann You still working on this? I guess having an instantiable EBayr::Client class with the same API as the module makes sense. Not sure why including the module as-is in the class wouldn't work...

meismann commented 8 years ago

Actually, it is done (https://github.com/meismann/ebayr/tree/multiple-instances) and I have been using it for my client since a few days. I need my current PR merged before I can open another one. AFAIK, if I merge my multiple-instances branch in my master, it would be in the current PR automatically.

meismann commented 8 years ago

Oh, and I need to update the README. So, it is not completely done, admittedly ;-)

bjjb commented 8 years ago

Cool. How about we merge it into the v0.1 branch, and release a gem in the next week or so?

meismann commented 8 years ago

All right, agreed!

bjjb commented 6 years ago

@meismann I'd like this feature, but it can't be merged standalone into master as-is. Did you base it on your local fork? Could you to re-implement it against @GCorbel 's changes?