Reworked @bryanlandia extension to allow application-only authentication

[jschultz]

I tidied up bryanlandia@ b6a427a a little, notably to remove the requirement to provide an access token when using application-only authentication. Also changed (removed) exception trapping, to make the changes consistent with existing code.

---

This change is 

[jeremylow]

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.

---

twitter/api.py, line 263 at r1 (raw file):

            ))

        if consumer_key is None or consumer_secret is None or (

Could we simplify this logic to:
if not all([consumer_key, consumer_secret, access_key, access_secret]) or application_only_auth or probably better:

if not all([consumer_key, consumer_secret, access_key, access_secret]) or all([consumer_key, consumer_secret, application_only_auth])

I think it accomplishes the same thing, but I could be totally wrong.

---

twitter/api.py, line 283 at r1 (raw file):

            requests_log.propagate = True

    def GetAppOnlyAuthToken(self, consumer_key, consumer_secret):

This should be four spaces.

---

Comments from Reviewable

[jschultz]

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.

---

twitter/api.py, line 263 at r1 (raw file):
The first test doesn't cover the case where application_only_auth is True but consumer_key or consumer_secret is None. Also both those expressions need an extra bracket around everything after not. I came up with the following, which is logically the same as your second, but IMO a bit clearer:

if not (all([consumer_key, consumer_secret])
   and (application_only_auth or all([access_token_key, access_token_secret]))):

---

twitter/api.py, line 283 at r1 (raw file):

Previously, jeremylow (Jeremy Low) wrote…

This should be four spaces.

Oops.

---

Comments from Reviewable

[jeremylow]

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion.

---

twitter/api.py, line 263 at r1 (raw file):

Previously, jschultz (Jonathan) wrote…

The first test doesn't cover the case where application_only_auth is True but consumer_key or consumer_secret is None. Also both those expressions need an extra bracket around everything after not. I came up with the following, which is logically the same as your second, but IMO a bit clearer:

if not (all([consumer_key, consumer_secret])
   and (application_only_auth or all([access_token_key, access_token_secret]))):

My concern with that is that it doesn't allow for creating an API instance without keys:

In [1]: import twitter

In [2]: api = twitter.Api()
---------------------------------------------------------------------------
TwitterError                              Traceback (most recent call last)
<ipython-input-2-7b2e0eb4f484> in <module>()
----> 1 api = twitter.Api()

/home/jeremy/coding/contrib/python-twitter/twitter/api.py in __init__(self, consumer_key, consumer_secret, access_token_key, access_token_secret, application_only_auth, input_encoding, request_headers, cache, base_url, stream_url, upload_url, chunk_size, use_gzip_compression, debugHTTP, timeout, sleep_on_rate_limit, tweet_mode)
    261         if not (all([consumer_key, consumer_secret])
    262            and (application_only_auth or all([access_token_key, access_token_secret]))):
--> 263             raise TwitterError({'message': "Missing oAuth Consumer Key or Access Token"})
    264 
    265         self.SetCredentials(consumer_key, consumer_secret, access_token_key, access_token_secret,

TwitterError: {'message': 'Missing oAuth Consumer Key or Access Token'}

whereas the current behavior is to allow that unless someone specifies a consumer_key:

In [1]: import twitter

In [2]: api = twitter.Api()

In [3]: api = twitter.Api(consumer_key='test')
Twitter now requires an oAuth Access Token for API calls. If you're using this library from a command line utility, please run the included get_access_token.py tool to generate one.
---------------------------------------------------------------------------
TwitterError                              Traceback (most recent call last)
<ipython-input-3-325909a88ca7> in <module>()
----> 1 api = twitter.Api(consumer_key='test')

/home/jeremy/coding/contrib/python-twitter/twitter/api.py in __init__(self, consumer_key, consumer_secret, access_token_key, access_token_secret, input_encoding, request_headers, cache, base_url, stream_url, upload_url, chunk_size, use_gzip_compression, debugHTTP, timeout, sleep_on_rate_limit, tweet_mode)
    262                   'generate one.', file=sys.stderr)
    263 
--> 264             raise TwitterError({'message': "Twitter requires oAuth Access Token for all API access"})
    265 
    266         self.SetCredentials(consumer_key, consumer_secret, access_token_key, access_token_secret)

TwitterError: {'message': 'Twitter requires oAuth Access Token for all API access'}

---

Comments from Reviewable

[jeremylow]

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[jschultz]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

twitter/api.py, line 263 at r1 (raw file):

Previously, jeremylow (Jeremy Low) wrote…

My concern with that is that it doesn't allow for creating an API instance without keys:

In [1]: import twitter

In [2]: api = twitter.Api()
---------------------------------------------------------------------------
TwitterError                              Traceback (most recent call last)
<ipython-input-2-7b2e0eb4f484> in <module>()
----> 1 api = twitter.Api()

/home/jeremy/coding/contrib/python-twitter/twitter/api.py in __init__(self, consumer_key, consumer_secret, access_token_key, access_token_secret, application_only_auth, input_encoding, request_headers, cache, base_url, stream_url, upload_url, chunk_size, use_gzip_compression, debugHTTP, timeout, sleep_on_rate_limit, tweet_mode)
    261         if not (all([consumer_key, consumer_secret])
    262            and (application_only_auth or all([access_token_key, access_token_secret]))):
--> 263             raise TwitterError({'message': "Missing oAuth Consumer Key or Access Token"})
    264 
    265         self.SetCredentials(consumer_key, consumer_secret, access_token_key, access_token_secret,

TwitterError: {'message': 'Missing oAuth Consumer Key or Access Token'}

whereas the current behavior is to allow that unless someone specifies a consumer_key:

In [1]: import twitter

In [2]: api = twitter.Api()

In [3]: api = twitter.Api(consumer_key='test')
Twitter now requires an oAuth Access Token for API calls. If you're using this library from a command line utility, please run the included get_access_token.py tool to generate one.
---------------------------------------------------------------------------
TwitterError                              Traceback (most recent call last)
<ipython-input-3-325909a88ca7> in <module>()
----> 1 api = twitter.Api(consumer_key='test')

/home/jeremy/coding/contrib/python-twitter/twitter/api.py in __init__(self, consumer_key, consumer_secret, access_token_key, access_token_secret, input_encoding, request_headers, cache, base_url, stream_url, upload_url, chunk_size, use_gzip_compression, debugHTTP, timeout, sleep_on_rate_limit, tweet_mode)
    262                   'generate one.', file=sys.stderr)
    263 
--> 264             raise TwitterError({'message': "Twitter requires oAuth Access Token for all API access"})
    265 
    266         self.SetCredentials(consumer_key, consumer_secret, access_token_key, access_token_secret)

TwitterError: {'message': 'Twitter requires oAuth Access Token for all API access'}

OK, changing existing behaviour as little as possible is good practice. But OTOH what is the sense of creating an API instance without any keys? That is, if it's just going to cause an error later on, isn't it better to trap the error as soon as it can be detected?

---

Comments from Reviewable

[jeremylow]

Since this is a point release, let's leave the logic as in master and we can set up an issue for 4.0. Sound fair?

[jeremylow]

---

Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[jeremylow]

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[jschultz]

Arghhh I was trying to make a new pull request but the change (Added GetStatuses method) seems to have been added to this pull request. I think I'll need several lifetimes before I can use git without making this kind of blunder.

[EDIT] I think I've managed to roll back the unwanted commit. Sorry for the mess...

[jeremylow]

fwiw, I like to do feature branches to keep things straight. One tool I use for that is git-flow

---

Review status: 0 of 1 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[jeremylow]

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[noelleleigh]

When should we expect to see a new release with this PR? Very interested in using Application-only Authentication.

[bear]

@noahleigh going to work on that tonight