python-amazon-simple-product-api icon indicating copy to clipboard operation
python-amazon-simple-product-api copied to clipboard

Handle multiple ResponseGroups

Open jbkkd opened this issue 10 years ago • 10 comments

The way this project currently works is by assuming there's only one response group ("Large"). The Product class is based on that response group, plus a few extras which are relevant to different ResponseGroups.

We should majorly refactor this to address different ResponseGroups and allow easy access to their fields (or Resposne Elements, as named by Amazon).

Reference for ResponseGroup can be found here, with the valid values: http://docs.aws.amazon.com/AWSECommerceService/latest/DG/ItemLookup.html

I'll be adding a few fields to the Product class meanwhile, but will try to iterate on creating a class that'll handle multiple response groups, or a class for each response group.

jbkkd avatar May 22 '15 08:05 jbkkd

Sounds interesting. Have you done any work on this?

yoavaviram avatar Apr 27 '16 11:04 yoavaviram

I've done some work but it's internal only currently, I hope I'll be able to open source it soon.

jbkkd avatar May 01 '16 09:05 jbkkd

Please share as soon as you feel it's ready.

yoavaviram avatar May 03 '16 08:05 yoavaviram

I would also be interested in this. I don't understand the xml well enough to make it happen, but I would be willing to help where I can.

JasonCrowe avatar May 05 '17 15:05 JasonCrowe

Ooops. Didn't see this and opened another feature request #133.

ThomasProctor avatar Mar 28 '18 20:03 ThomasProctor

Should these features be added as new methods (ex api.lookup_alternate_versions(...)) or using the ResponseGroup keyword argument?

ThomasProctor avatar Mar 28 '18 22:03 ThomasProctor

It looks like the most recent version handles the "Images" response group by adding the property "images", which can be used by using the ResponseGroup keyword argument.

ThomasProctor avatar Mar 28 '18 22:03 ThomasProctor

I went ahead and submitted a pull request for the features that I wanted to use. It's modeled on how we dealt with the "Images" response group.

Some of the other response groups might not be such low hanging fruit. The "Variations" response group, for instance, has a much more complex tree structure, which might be inconsistent across different searches. Maybe each variation will adapt well to the AmazonProduct class, and a variations property can return a list of AmazonProducts?

As @jbkkd mentioned, it might be worthwhile to just refactor the whole code base (Use different methods for each response group, and have each response have a different class?), but it didn't seem like that was going to happen any time soon.

ThomasProctor avatar Mar 28 '18 23:03 ThomasProctor

@jbkkd, would you be interested in attempting the re-factor / becoming a core committer?

yoavaviram avatar Mar 29 '18 16:03 yoavaviram

It might be a good idea to focus on documentation of how this works in the short term. If we mention somewhere which response groups currently work and which won't, that would go a long way towards improving the UI. Maybe the refactor won't be 100% necessary if we do that well. If we could warn or raise an error if a user passes an unsupported response group, that would probably help a lot too.

Right now, "BrowseNodeInfo", "Large", "Images", and "AlternateVersions" are supported explicitly. I imagine we could add support for "Small" and "Medium" by just writing some tests for them?

ThomasProctor avatar Mar 29 '18 17:03 ThomasProctor