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

support python 3 #40

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

support python 3 #40

wants to merge 7 commits into from

Conversation

gusibi
Copy link

@gusibi gusibi commented Aug 18, 2017

No description provided.

Copy link
Contributor

@liuchang0812 liuchang0812 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you! hope it could be merged as soon as possible. I will test it next week.

except ImportError:
from urllib.parse import quote # Python 3+

from .helper import smart_str, smart_bytes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure whether this importation is worked on Python3. Python3 supports absolute import only. It would be better to update to from qcloud_cos.helper import smart_str, smart_bytes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, I prefer to use

from __future__ import absolute_import
from qcloud_cos.helper import smart_str, smart_bytes

logger.debug("sending request, method: %s, bucket: %s, cos_path: %s" % (method, bucket, cos_path))
print('url' * 10)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not like print in library. using logging would be better. 👍

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleted

@@ -105,6 +113,7 @@ def send_request(self, method, bucket, cos_path, **kwargs):
http_resp = self._http_session.get(url, verify=False, **kwargs)

status_code = http_resp.status_code
print(http_resp, http_resp.headers)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

:param param_name: param_name 参数名
:param param_value: param_value 参数值
:return:
"""
if param_value is None:
self._err_tips = param_name + ' is None!'
return False
if not isinstance(param_value, unicode):
if six.PY3:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

six has a type alias called string_types, could we use it here?

@@ -555,41 +557,26 @@ def check_params_valid(self):
if not super(UpdateFileRequest, self).check_params_valid():
return False

check_unicode_params = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I like it.

setup.py Outdated
@@ -4,9 +4,6 @@
import sys
from setuptools import setup, find_packages

if sys.version_info[0] != 2 or sys.version_info[1] < 6:
sys.exit('Sorry, only python 2.6 or 2.7 is supported')

setup(name='qcloud_cos_v4',
version='0.0.20',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to update version to 1.0.0 if we could merge this. : )

local_file_2.txt Outdated
@@ -0,0 +1 @@
hello world2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can't find where it is used. is it a tmp file?

@gusibi
Copy link
Author

gusibi commented Aug 18, 2017

Auth 添加 appid, secret_id, secret_key 参数,以便可以单独使用

@liuchang0812
Copy link
Contributor

请添加 six 依赖

@liuchang0812
Copy link
Contributor

tox.ini 中也要添加 six 依赖

@gusibi
Copy link
Author

gusibi commented Aug 24, 2017

没有找到这个文件

@liuchang0812
Copy link
Contributor

忽略tox,回归的错误修一下吧 https://travis-ci.org/tencentyun/cos-python-sdk-v4/jobs/266680039

@gusibi
Copy link
Author

gusibi commented Sep 19, 2017

什么时候可以 merge?

@ImSingee
Copy link

@liuchang0812 希望尽快 Merge

@liuchang0812
Copy link
Contributor

@dt3310321

@gusibi
Copy link
Author

gusibi commented Dec 5, 2017

😲 还没 review 。。。

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.

3 participants