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

16 June 2015 - Replace "liked" with "upvoted" for all purposes #429

Closed
wants to merge 2 commits into from
Closed

16 June 2015 - Replace "liked" with "upvoted" for all purposes #429

wants to merge 2 commits into from

Conversation

voussoir
Copy link
Contributor

I just realized that praw never received this update. Reddit isn't closing "liked" and "disliked" yet, but it will probably happen eventually.

http://www.reddit.com/r/changelog/comments/36t36j/reddit_change_all_remaining_uses_of_liked_and/

Added methods get_upvoted and get_downvoted to objects.Redditor, keeping get_liked and get_disliked as shortcut methods with a note in the docstring. I didn't think a full-blown deocrators.deprecated warning was necessary, since the functionality doesn't change at all.

 

I'm not sure what's going on with test_oauth2_reddit.test_scope_history. When I run py setup.py test I see this:

test_scope_creddits (tests.test_oauth2_reddit.OAuth2RedditTest) ... ERROR
test_scope_edit (tests.test_oauth2_reddit.OAuth2RedditTest) ... ERROR
test_scope_history (tests.test_oauth2_reddit.OAuth2RedditTest) ... ok
test_scope_identity (tests.test_oauth2_reddit.OAuth2RedditTest) ... ERROR
test_scope_modconfig (tests.test_oauth2_reddit.OAuth2RedditTest) ... ERROR

The Travis error log is inverted -- all ok except for test_scope_history. What does this mean?

The "liked" and "disliked" names will eventually be phased out. Add
methods get_upvoted and get_downvoted to objects.Redditor, keeping
get_liked as a pointer.
Fixed test_scope_history to use Redditor.get_upvoted
@bboe
Copy link
Member

bboe commented Jun 26, 2015

I'll try taking a look at this in a bit so we can get this merged in.

@bboe
Copy link
Member

bboe commented Jun 26, 2015

So here's what I'm assuming is why the tests passed in an inverted state. I'm guessing you have a ~/.config/praw.ini file that has a reddit_oauth_test section with an oauth_redirect_uri that begins with http://. The built-in praw.ini now uses https:// for that setting. When the setting changes, the requests that are made in the test-suite changes hence only the test you updated worked for you, and that happened to be the only one that failed on travis.

Either removing the reddit_oauth_test in your praw.ini file, or updating them to match should prevent similar issues in the future.

Merged as: 2ea38cb

Thanks!

@voussoir
Copy link
Contributor Author

The praw.ini file in my branch matches the one in the master repo 100%, so that can't be it. Could it have something to do with the oauth tokens needing to be regularly refreshed, and maybe the cassette I generated was outdated by the time Travis ran the test? I don't know enough about oauth to make any better guesses.

@bboe
Copy link
Member

bboe commented Jun 26, 2015

The oauth tokens I think only ever needed to be refreshed once (if at all). My guess is you have a praw.ini file somewhere that you're not aware of. This is the logic for where all to find praw.ini files: https://github.com/praw-dev/praw/blob/master/praw/settings.py#L28

Try running:

import praw
r = praw.Reddit('tmp', 'reddit_oauth_test')
print(r.config.redirect_uri)

Does the output url match: https://127.0.0.1:65010/authorize_callback?

@voussoir
Copy link
Contributor Author

You're right, there's a copy from February 2014 in AppData/Roaming, haha.

Looks like the config loader does a for-loop over all the filepaths, overwriting the dictionary values each time. Because the appdata file was the last to load, these settings became finalized. Changing the locations.insert() position from 1 to 0 here solved that.

You might want to consider making this change in the repo so that local files always land on top.

edit: The third entry in the locations list is the CWD, so that's good. It is only the first and second values that need to swap so module_dir is after config_path.

@bboe
Copy link
Member

bboe commented Jun 26, 2015

That order is by design if you look at the documents. It allows you to overwrite the global ini if you want to. Generally you wouldn't use it to redefine any sections other than the reddit one though. You can always add more new config sections. Does that make sense?

@bboe
Copy link
Member

bboe commented Jun 27, 2015

something to do with the oauth tokens needing to be regularly refreshed

I see what you mean here. That could be a problem. However, betamax captures the request to obtain a new access token, and thus always returns the same one in replayed tests.

@voussoir
Copy link
Contributor Author

Yeah, you're right about that, the http line must have been the problem.

I guess I see your point about the config locations. In my case, the CWD was the praw folder itself, so I was expecting it to load that config instead of jumping out to AppData. If I had been anywhere else it would have made more immediate sense. Most people don't work from within their python Lib, so I suppose they won't be running into that issue.

@voussoir voussoir deleted the praw-upvoted branch June 28, 2015 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants