10
\$\begingroup\$

This is a script which takes an Amazon URL as input, takes out ASIN/ISBN from the URL, and uses Amazon Python API to fetch details.

For this task, I looked at many Amazon URLs and observed the following things:

  1. ASIN/ISBN is always present in an Amazon URL
  2. ASIN/ISBN can be present anywhere in the URL
  3. ASIN is always in capital letters, consists of alphabets and numbers only and length is always 10
  4. If ASIN is not present then it's a link to a book on Amazon and ISBN will be present
  5. ISBN consists only of numbers
  6. Amazon uses ISBN-10 format i.e. length of ISBN is 10
  7. Haven't come across ASIN which is purely numbers and without any alphabets
  8. ASIN or ISBN, always followed by '/'

I hope I covered everything, but if I missed some case, please let me know.

Secondly, I care about only Amazon India. So it throws that ASIN is invalid if I give an ASIN of Amazon US or any other country, or gives pricing info zero. I don't know why it is causing such behaviour (example).

Flow: Get the URL, check for either ASIN or ISBN, if none are present return false else use API to fetch name, price, URL and product image.

import re
from amazon.api import AmazonAPI

AMAZON_ACCESS_KEY = 'AKI...JP2'
AMAZON_SECRET_KEY = 'Eto...uxV'
AMAZON_ASSOC_TAG = 'p...1'

asin_regex = r'/([A-Z0-9]{10})'
isbn_regex = r'/([0-9]{10})'

def get_amazon_item_id(url):
    # return either ASIN or ISBN
    asin_search = re.search(asin_regex, url)
    isbn_search = re.search(isbn_regex, url)
    if asin_search:
        return asin_search.group(1)
    elif isbn_search:
        return isbn_search.group(1)
    else:
        # log this URL
        return None

def get_amazon_product_meta(url):
    # the input URL is always of amazon
    amazon = AmazonAPI(AMAZON_ACCESS_KEY, AMAZON_SECRET_KEY, AMAZON_ASSOC_TAG, region="IN")

    item_id = get_amazon_item_id(url)
    if not item_id:
        return None

    try:
        product = amazon.lookup(ItemId=item_id)        
    except amazon.api.AsinNotFound:
        # log this ASIN
        return None

    # product.price_and_currency returns in the form (price, currency)
    product_price = product.price_and_currency[0]

    if product_price:
        return product.offer_url, product.title, product.large_image_url, product_price
    return None
\$\endgroup\$
7
\$\begingroup\$

This should be a short review mainly because your code looks pretty nice.

I don't have any experience with the Amazon API, but from what I can tell you are using it as I would expect it to be used.

The only points that I want to speak on and can nit-pick are:

  1. Your variables asin_regex and isbn_regex are slightly misleading. In Python there is a distinction between regular expression patterns and regular expression objects. The suffix _regex implies that, by itself, the variable can recognize expressions which, in turn, implies the variable is a regular expression object. Because your variables cannot, by themselves, recognize expressions, I would use the suffix _pattern.
  2. Your logic in get_amazon_id is completely fine. Its my personal preference, in this case where you are simply returning inside the if blocks, to just use if statements, no elif or else blocks:

    if asin_search:
        return asin_search.group(1)
    if isbn_search:
        return isbn_search.group(1)
    
    return None
    

    The above is strictly personal preference, simply getting another version out there.

    However, what I would recommend doing is use a for loop to reduce some of the minor code repetition and make your code more flexible:

    for search in asin_search, isbn_search:
        if search:
            return search.group(1)
    return None
    

    The above code would be best if, say, for some reason another ID type came out. So, instead of having to add another if statement, all you need to do is add new_id_search to the for loop list.

  3. I would change your regex patterns to use the \w and \d characters:

    asin_regex = r'/(\w{10})'
    isbn_regex = r'/(\d{10})' 
    

    The \d character would just make your currently short regex even shorter, while the \w character helps protect against the case that for some unknown reason the ASIN or ISBN contains lower-case characters.

  4. Finally, I would capitalize the first word of all your comments (unless it is a variable name).

\$\endgroup\$
  • \$\begingroup\$ Thank you very much! ASIN is always in capital letters and it would never contain small letters. And also r'/(\w{10})' would match 'BlackBerry', but product names URL are never fully capitalised. \$\endgroup\$ – avi Jul 9 '14 at 13:27
  • \$\begingroup\$ I think if: A elif: B else: C is clearer than if: A if: B C even when A and B are return statements. \$\endgroup\$ – Gareth Rees Jul 9 '14 at 13:32
  • \$\begingroup\$ @GarethRees As I said, that point is personal preference. I can see both ways being attractive. \$\endgroup\$ – BeetDemGuise Jul 9 '14 at 13:34
  • \$\begingroup\$ @avi Good point about the pattern matching 'BlackBerry'. I am still wary of implied capitalization. However, the problem of possible lowercase letters may just be a handle-it-if-it-comes-up problem. \$\endgroup\$ – BeetDemGuise Jul 9 '14 at 13:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.