bear / python-twitter

A Python wrapper around the Twitter API.
Apache License 2.0
3.41k stars 957 forks source link

URL shortening parameter is not used in api.py, therefore service is not available #298

Closed immanuelfactor closed 8 years ago

immanuelfactor commented 8 years ago

Hi bear, You've did a nice job on this repo! :) Please correct me if I'm wrong but I can't use the custom URL shortening service as-is because there is no reference of it in your api.py apart from a parameter and a documentation comment: https://github.com/bear/python-twitter/blob/master/twitter/api.py#L140 https://github.com/bear/python-twitter/blob/master/twitter/api.py#L169 I have implemented after some work a bitly oauth client which can shorten urls, then created a url shortener class according to your shorten_url.py but when I tried to use it, there was no effect of it. When I examined the api.py, I've recognized the shortner variable is not used, which is a really great downside of the whole project. Can you or somebody else please provide a fix for this issue? :)

immanuelfactor commented 8 years ago

I've came up with a possible solution (quickfix). Instead of using the incomplete API shortening function, I replace the URL before the message is passed to the API. The below code handles only one URL per tweet but it suits my needs now.

A new file, called __init__.py needs to be placed in the examples folder to allow importing our classes. Changes in tweet.py:

# two new import lines for regex and shortening class
import re
from shortenclassfile import ShortenURL # replace the class file name

# new lines in main(), after the api = twitter.Api( ... line
    shortner = ShortenURL()
    if 'http' in message:
        long_url = re.search("(?P<url>https?://[^\s]+)", message).group("url")
        print "An URL found, shortening:", long_url
        short_url = shortner.Shorten(long_url)
        print "Shortening done:", short_url
        message = message.replace(long_url, short_url)
# the following line is the current try: PostUpdate block

# two new sys lines fixing the replace constantly falling back to 'ascii' and throwing error
# @see: http://stackoverflow.com/a/21190382
if __name__ == "__main__":
    reload(sys)
    sys.setdefaultencoding('utf8')
    main()

I don't want to make a pull request for this because the perfect solution would be really using the shortner parameter of the api class.

jeremylow commented 8 years ago

If you have a working implementation of a shortener service, you can create a shortener instance, pass the URL to that, and replace the long URL with the short URL in the status string, and then post the modified string to Api.PostUpdate():

import re

from twitter import Api
from twitter.twitter_utils import URL_REGEXP
from shorten_url import ShortenURL

CONSUMER_KEY = 'WWWWWWW'
CONSUMER_SECRET = 'XXXXXXXXXXX'
ACCESS_TOKEN = 'YYYYYYYYYYY'
ACCESS_TOKEN_SECRET = 'ZZZZZZZZZ'

def PostStatusWithShortenedURL(status):
    shortener = ShortenURL()
    api = Api(CONSUMER_KEY,
              CONSUMER_SECRET,
              ACCESS_TOKEN,
              ACCESS_TOKEN_SECRET)

    urls = re.findall(URL_REGEXP, status)

    for url in urls:
        status = status.replace(url, shortener.Shorten(url), 1)

    api.PostUpdate(status)

if __name__ == '__main__':
    PostStatusWithShortenedURL("this is a test: http://www.example.com/tests")
jeremylow commented 8 years ago

Just saw your update. The above implementation allows for more than one shortened URL and finds more URLs (for example, if they don't start with http).

immanuelfactor commented 8 years ago

Your solution is really elegant, thank you! :)

immanuelfactor commented 8 years ago

Works like a charm! :) Proof: http://fodor.it/1Xxzbje

jeremylow commented 8 years ago

As for reintroducing the shortner parameter to Twitter, my thinking is that this should remain up to the end user. My reasoning goes like this:

  1. You're already constructing an object to attach to the Api; it's not that far from there to apply the code above to shorten any URLs in the status you're going to be posting to be passed to PostUpdate().
  2. Shortening a URL is a business decision (primarily), so your business logic should control and I don't think it's the place of the Api to determine how and when that happens, even if you're able to control which service is used.
  3. I think it's prudent to consider that not all URLs should get shortened, but with an Api-wide shortener, this isn't really an option: you have to make a choice on instantiation and then you can't really change it. (Do you shorten all URLs or only those you control? Just breaking news from your site or from your competitors as well? What about longform where link-rot could play a factor and you might not want to rely on an external service?)
  4. The logic of no. 3 gets even weirder when(/if) Twitter/Google rolls out their version of Instant Articles and then that's even more of a mess since, depending on the implementation, you're going to be dealing a whole new class of decisions as to whether something should get shortened or not.
  5. Shortening some URLs breaks the UI of modern Twitter clients. For example: if you shorten all URLs, then you're going to be shortening quote-replies, which are, if not ubiquitous, really popular; having an Api-wide shortener that you can't easily break out of would shorten URLs like https://twitter.com/jack/status/129301823 and the quote-reply wouldn't work. There's no way that I can see (without hard-coding exceptions) to avoid this situation.

Anyway, those are just my thoughts on the subject, but I thought I should address it, though I don't want to speak for @bear or anybody else. I guess my preference would have this as something in that should go in the documentation so there's a reference and how-to and then remove the parameter from the API.

bear commented 8 years ago

I found my self nodding in agreement - giving the user of python-twitter the power to decide and the tools to do that makes more sense IMO. We will never be able to think about all the edge cases.

immanuelfactor commented 8 years ago

I'm totally fine with the idea of removing the shortening from the API's core features. However, I insist on keeping the shortener example file, and putting the above @jeremylow example to a separate file or in the tweet.py example or in the shorten_url.py itself. This is a nice feature to keep and let people use it if we've already have it in place. In this ticket, it would get forgotten and unnoticed by many.

jeremylow commented 8 years ago

Just created a PR for the documentation and examples, so I think the issue can be closed. Code from above is now in the shorten_url.py example and has inline comments so folks can see how it works a little easier.