Pipnize drucker #6
Conversation
keigohtr
requested review from
sugyan,
yoquankara,
yustoris,
jkw552403,
yuki-mt and
shimpei-yotsukura
|
|
| db_name = "test_"+config.DB_MYSQL_DBNAME if config.TEST_MODE else config.DB_MYSQL_DBNAME | ||
| user = config.DB_MYSQL_USER | ||
| password = config.DB_MYSQL_PASSWORD | ||
| url = f'mysql+pymysql://{user}:{password}@{host}:{port}/{db_name}?charset=utf8' |
There was a problem hiding this comment.
This function uses f-string twice and only Python 3.6+ support f-string. Should we change these two lines to support python 3.4 and 3.5?
There was a problem hiding this comment.
Sounds good!
Before committing the change mentioned above, please confirm all unittests pass correctly.
| - python: 3.6 | ||
| env: TOXENV=py36 | ||
| - python: 3.6 | ||
| env: TOXENV=coverage,codecov |
There was a problem hiding this comment.
Should we add python 3.7? I think 3.7 is the latest major release and the code looks fine with the latest version.
There was a problem hiding this comment.
Yes, there was such a problem. But it only happens with older versions of kubernetes-client, and seems to work well with the current version.
- https://github.com/kubernetes-client/python/blob/master/CHANGELOG.md#v700a1
- feat: support for 3.7 (workaround) kubernetes-client/gen#67
Anyway, this repository does not use kubernetes-client, so it seems that there is no problem even adding 3.7 to support targets.
There was a problem hiding this comment.
Thank you @sugyan san!
OK, let’s support Python 3.7.
|
What is blocking this PR? |
|
@yoquankara There is no blocking stuffs. |
|
I see, thank you. LGTM |
What is this PR for?
To gain the usability, we have developed pipnized Drucker.
Note
This PR includes
What type of PR is it?
Feature
What is the issue?
#2
How should this be tested?
Example