Make 37 P2P wrapper python 3.7 compatible#45
Make 37 P2P wrapper python 3.7 compatible#45sosdmike wants to merge 5 commits intodatadesk:masterfrom
Conversation
|
This is relevant to my interests. One thought: can we just run the whole project through Black first, so there aren't any whitespace changes to diff? |
|
@carlmjohnson The codebase as it stands is fairly consistent so it might be better to make an Issue to make the entire codebase pep8/black/flake8/whatever compliant. As it stands with this Merge request there are only 2 lines of whitespace changed. |
|
Is anyone from the LAT still using reviewing this repo? |
|
Yes. I use it and I'm an admin. But I'm not connected to the SNAP project anymore. Since this wrapper is, as far as I know, still a critical dependency there I'm reluctant to make sudden moves on my own. If we limit this ticket to strictly Python 3.7 support I think I can merge it for you. |
|
The point of suggesting Black was just to make the diff here smaller by putting all the formatting changes into a separate 100% mechanical PR. If you can review the changes as they are, it isn't necessary. |
|
@palewire Mike is part of the SNAP project and is aware of it's dependency on the wrapper. I wanted to make sure that supporting python 3 wouldn't step on any datadesk projects. For safety I'll review the PR today as well. |
|
If it works for you, I'm sure it'll work for me.
…On Thu, Aug 23, 2018, 8:20 AM James Perez ***@***.***> wrote:
@palewire <https://github.com/palewire> Mike is part of the SNAP project
and is aware of it's dependency on the wrapper. I wanted to make sure that
supporting python 3 wouldn't step on any datadesk projects. For safety I'll
review the PR today as well.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#45 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAnCVmbJfFjkqCp05DqYGK39hgjYlHKks5uTsgpgaJpZM4Vyp8B>
.
|
|
@palewire Can this branch be merged and a new version pushed to pypi? |
|
|
||
| # Get alt_thumbnail_url and old_slug for thumbnail logic below | ||
| alt_thumbnail_url = content_item.get('alt_thumbnail_url') | ||
| # alt_thumbnail_url = content_item.get('alt_thumbnail_url') |
| log.debug("[P2P][RESPONSE] %s" % request_log) | ||
|
|
||
| if resp.status_code >= 500: | ||
| response_text = resp.text |
There was a problem hiding this comment.
I'm sure there's a good reason, but why are we moving from resp.content to resp.text?
| p2p = get_connection() | ||
| test_story_slugs = ["la-test-p2p-python-temp-story-%s" % x for x in range(0, 8)] | ||
| first_test_story_slug = "la-test-p2p-python-temp-story-0" | ||
| eighth_test_story_slug = "la-test-p2p-python-temp-story-7" |
There was a problem hiding this comment.
What's the goal of this new test story?
4 participants
No description provided.