Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
immanuelfactor opened this Issue Feb 16, 2016 · 9 comments

Comments

Projects
None yet
3 participants
@immanuelfactor
Copy link

immanuelfactor commented Feb 16, 2016

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

This comment has been minimized.

Copy link
Author

immanuelfactor commented Feb 17, 2016

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

This comment has been minimized.

Copy link
Collaborator

jeremylow commented Feb 17, 2016

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

This comment has been minimized.

Copy link
Collaborator

jeremylow commented Feb 17, 2016

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

This comment has been minimized.

Copy link
Author

immanuelfactor commented Feb 17, 2016

Your solution is really elegant, thank you! :)

@immanuelfactor

This comment has been minimized.

Copy link
Author

immanuelfactor commented Feb 17, 2016

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

@jeremylow

This comment has been minimized.

Copy link
Collaborator

jeremylow commented Feb 18, 2016

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

This comment has been minimized.

Copy link
Owner

bear commented Feb 18, 2016

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

This comment has been minimized.

Copy link
Author

immanuelfactor commented Feb 18, 2016

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 added a commit that referenced this issue Feb 18, 2016

@jeremylow

This comment has been minimized.

Copy link
Collaborator

jeremylow commented Feb 18, 2016

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.

@jeremylow jeremylow closed this Feb 18, 2016

jeremylow added a commit that referenced this issue May 8, 2016

Merge pull request #333 from bear/fixes/issue298
Removes shortner param from API per #298
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.