#34 Use poetry, requirements, put in a module ready for publication, bump to 0.6.2

Closed
rigelk wants to merge 13 commits from rigelk/prismedia:requirements into develop
rigelk commented 5 years ago

A bunch of things :)

With this, we have:

  • locked dependencies
  • compatibility with pip via generated requirements.txt from the pyproject.toml (new standard PyPa format)
  • a publishable prismedia module (you just have to do a poetry publish…)
  • the module installs prismedia as a command within the PATH, otherwise running prismedia.upload behaves like the current script.
A bunch of things :) With this, we have: - locked dependencies - compatibility with pip via generated requirements.txt from the pyproject.toml (new standard PyPa format) - a publishable `prismedia` module (you just have to do a `poetry publish`…) - the module installs `prismedia` as a command within the `PATH`, otherwise running `prismedia.upload` behaves like the current script.
rigelk commented 5 years ago
Poster

I just added:

  • support for python ^3.3 (tested up to 3.7.2).
  • moved config file samples to a in-module directory, with a submodule writing them if need be.
I just added: - support for python ^3.3 (tested up to 3.7.2). - moved config file samples to a in-module directory, with a submodule writing them if need be.
Poster
Owner

Thanks a lot for the work! I have began to test the code, let you know if there is problem ;-)

Hope to merge soon! 🙏

Thanks a lot for the work! I have began to test the code, let you know if there is problem ;-) Hope to merge soon! :pray:
LecygneNoir reviewed 5 years ago
LecygneNoir left a comment

For reference:

When installing poetry it complains about a difference between the .lock and the .toml files:

± poetry install
Installing dependencies from lock file
Warning: The lock file is not up to date with the latest changes in pyproject.toml. You may be getting outdated dependencies. Run update to update them.

However when running the update, it fails with an error:

[SolverProblemError]
The current project must support the following Python versions: ~2.7 || ^3.2
Because future (0.17.1) requires Python >=2.6, !=3.0., !=3.1., !=3.2.*
and no versions of future match >0.17.1,<0.18.0, future is forbidden.
So, because prismedia depends on future (^0.17.1), version solving failed.

Seems to me something is incorrect between versions needed by poetry somewhere.

I’ll need to learn how poetry works before reviewing this one, so feel free if you have any idea ;-)

README.md Outdated
Otherwise, you can use [poetry](https://poetry.eustace.io/):
```
poetry install # installs the dependency in the current virtualenv, or creates one specific to the project if no virtualenv is currently active
LecygneNoir commented 5 years ago

For reference:

When installing poetry it complains about a difference between the .lock and the .toml files:

± poetry install
Installing dependencies from lock file
Warning: The lock file is not up to date with the latest changes in pyproject.toml. You may be getting outdated dependencies. Run update to update them.

However when running the update, it fails with an error:

[SolverProblemError]                                             
The current project must support the following Python versions: ~2.7 || ^3.2  
Because future (0.17.1) requires Python >=2.6, !=3.0.*, !=3.1.*, !=3.2.*      
 and no versions of future match >0.17.1,<0.18.0, future is forbidden.        
So, because prismedia depends on future (^0.17.1), version solving failed.  

Seems to me something is incorrect between versions needed by poetry somewhere.

I'll need to learn how poetry works before reviewing this one, so feel free if you have any idea ;-)

For reference: When installing poetry it complains about a difference between the .lock and the .toml files: ``` ± poetry install Installing dependencies from lock file Warning: The lock file is not up to date with the latest changes in pyproject.toml. You may be getting outdated dependencies. Run update to update them. ``` However when running the update, it fails with an error: ``` [SolverProblemError] The current project must support the following Python versions: ~2.7 || ^3.2 Because future (0.17.1) requires Python >=2.6, !=3.0.*, !=3.1.*, !=3.2.* and no versions of future match >0.17.1,<0.18.0, future is forbidden. So, because prismedia depends on future (^0.17.1), version solving failed. ``` Seems to me something is incorrect between versions needed by poetry somewhere. I'll need to learn how poetry works before reviewing this one, so feel free if you have any idea ;-)
rigelk commented 5 years ago

ah. AH.

Right, that's my ^3.2 which was putting future off.

ah. AH. Right, that's my ^3.2 which was putting future off.
prismedia/pt_upload.py Outdated
from . import utils
PEERTUBE_SECRETS_FILE = 'peertube_secret'
LecygneNoir commented 5 years ago

As you have changed the directory of peertube_secret you need to update the path here too, eg

PEERTUBE_SECRETS_FILE = 'prismedia/config/peertube_secret'

As you have changed the directory of peertube_secret you need to update the path here too, eg `PEERTUBE_SECRETS_FILE = 'prismedia/config/peertube_secret'`
rigelk commented 5 years ago

no, as this is the name of the file in the current directory, not the template directory.

no, as this is the name of the file in the current directory, not the template directory.
LecygneNoir commented 5 years ago

In this case we need to indicate somewhere that we need to launch prismedia from the config directory

From my test, I ran it from the project root, and it break directly. By default it seems to me than people will run it from the root of the project 🤔

(It makes less sense with the fact that this is a real python module however...)

In this case we need to indicate somewhere that we need to launch prismedia from the config directory From my test, I ran it from the project root, and it break directly. By default it seems to me than people will run it from the root of the project :thinking: (It makes less sense with the fact that this is a real python module however...)
LecygneNoir commented 5 years ago

Okay nevermind, as discussed the fact that it is now a module that can be runned anywhere means we cannot assume prismedia will be launched from the project repository anymore.

It makes the use of python -m prismedia.genconfig. more necessary, and I need to think about some description inside the changelog to explain the change to people already using prismedia 👍

Okay nevermind, as discussed the fact that it is now a module that can be runned anywhere means we cannot assume prismedia will be launched from the project repository anymore. It makes the use of `python -m prismedia.genconfig.` more necessary, and I need to think about some description inside the changelog to explain the change to people already using prismedia :thumbsup:
LecygneNoir commented 5 years ago

It seems ConfigParser and configparser does not behave the same 🤔

Indeed when parsing the NFO, configparser causes an error as it not translate parameters into string:

2019-02-21 07:08:06,587 Using prismedia/config/nfo_example.txt as NFO, loading...
Key '--playlist' error:
Or(None, <type 'str'>) did not validate u'My Test Playlist'
u'My Test Playlist' should be instance of 'str'

(Using the default nfo_example)

When rollbacking to the previous ConfigParser it works again.

I am testing with python2.7.15, will need to dig to see what the problem is, probably we need more options with configparser to specify what is a string.

It seems `ConfigParser` and `configparser` does not behave the same :thinking: Indeed when parsing the NFO, `configparser` causes an error as it not translate parameters into string: ``` 2019-02-21 07:08:06,587 Using prismedia/config/nfo_example.txt as NFO, loading... Key '--playlist' error: Or(None, <type 'str'>) did not validate u'My Test Playlist' u'My Test Playlist' should be instance of 'str' ``` (Using the default nfo_example) When rollbacking to the previous `ConfigParser` it works again. I am testing with `python2.7.15`, will need to dig to see what the problem is, probably we need more options with `configparser` to specify what is a string.
LecygneNoir commented 5 years ago

Mmmh according to the documentation the rename occured in python2 -> python3 migration:

https://docs.python.org/2/library/configparser.html

But in python2 it does not complains about an incorrect lib, so it knowns configparser as well.

However python3 documentation states than Config parsers do not guess datatypes of values in configuration files, always storing them internally as strings. (same than old ConfigParser) but it's obviously not the case using python2 :-/

So the lib is recognized in python2, but seems broken.

I am unfamiliar with migration from python2 to python3, if you have any idea how we could deal with it?
Perhaps some if statement to change import regarding the version?

Mmmh according to the documentation the rename occured in python2 -> python3 migration: https://docs.python.org/2/library/configparser.html But in python2 it does not complains about an incorrect lib, so it knowns `configparser` as well. However python3 documentation states than `Config parsers do not guess datatypes of values in configuration files, always storing them internally as strings. ` (same than old `ConfigParser`) but it's obviously not the case using python2 :-/ So the lib is recognized in python2, but seems broken. I am unfamiliar with migration from python2 to python3, if you have any idea how we could deal with it? Perhaps some if statement to change import regarding the version?
rigelk commented 5 years ago

I corrected the implementation to properly type instances of strings.

I corrected the implementation to properly type instances of strings.
LecygneNoir commented 5 years ago

I confirm that problem is solved for nfo, thanks!!

I confirm that problem is solved for nfo, thanks!!
http.client.BadStatusLine,
)
RETRIABLE_STATUS_CODES = [500, 502, 503, 504]
LecygneNoir commented 5 years ago

Out of the diff, but at lines 53 and 54 you need to append the new config directory to the secrets files

CLIENT_SECRETS_FILE = 'prismedia/config/youtube_secret.json'
CREDENTIALS_PATH = "prismedia/config/.youtube_credentials.json"
Out of the diff, but at lines 53 and 54 you need to append the new config directory to the secrets files ``` CLIENT_SECRETS_FILE = 'prismedia/config/youtube_secret.json' CREDENTIALS_PATH = "prismedia/config/.youtube_credentials.json" ```
rigelk commented 5 years ago

no, as this is the name of the file in the current directory, not the template directory.

no, as this is the name of the file in the current directory, not the template directory.
LecygneNoir added the
enhancement
label 5 years ago
LecygneNoir added the
Work in Progress
label 5 years ago
rigelk commented 5 years ago
Poster

Should be good to test (and hopefully good to merge too!)

Should be good to test (and hopefully good to merge too!)
LecygneNoir reviewed 5 years ago
2.7.15
LecygneNoir commented 5 years ago

This file seems to broke poetry when not using this exact version.

I load the poetry virtualenv, then cd in the repo, and immediatly the virtualenv deactivate :-/

Should it be a test file added in the .gitignore?

This file seems to broke poetry when not using this exact version. I load the poetry virtualenv, then cd in the repo, and immediatly the virtualenv deactivate :-/ Should it be a test file added in the .gitignore?
rigelk commented 5 years ago

It is not necessary anymore - it's an artifact from using pyenv to force python 2 when that was all we supported…

It is not necessary anymore - it's an artifact from using `pyenv` to force python 2 when that was all we supported…
import logging
import datetime
import pytz
from os.path import splitext, basename, abspath
LecygneNoir commented 5 years ago

I notice that at the beginning of the libs we still use #!/usr/bin/env python2 as shebang.

It does nothing as files are never direclty calles as script, but to avoid any future (and hard to debug :D) problems between python2 and python3, it should be better to totally remove them?

Sheband are not needed when loading python files as libraries?

I notice that at the beginning of the libs we still use `#!/usr/bin/env python2` as shebang. It does nothing as files are never direclty calles as script, but to avoid any future (and hard to debug :D) problems between python2 and python3, it should be better to totally remove them? Sheband are not needed when loading python files as libraries?
rigelk commented 5 years ago

Yes, we should remove them, libraries don't use them.

Yes, we should remove them, libraries don't use them.
def run(options):
secret = ConfigParser()
try:
secret.read(PEERTUBE_SECRETS_FILE)
LecygneNoir commented 5 years ago

There is another problem with configparser later in the script.

It's also used to load the peertube_secret, and it's broken if the peertube password contains special char (for example, %)

The exact error is:

2019-02-22 07:34:13,492 Peertube: Error: '%' must be followed by '%' or '(', found: "%xR))'2"

(of course the password is not a real one :p)

To reproduce:

  • use a password with % inside peertube_secret
  • upload to peertube only
python -m prismedia --file="path/files.mp4" --platform=peertube

You should not need a valid peertube access, as it breaks before upload.

There is another problem with configparser later in the script. It's also used to load the peertube_secret, and it's broken if the peertube password contains special char (for example, %) The exact error is: `2019-02-22 07:34:13,492 Peertube: Error: '%' must be followed by '%' or '(', found: "%xR))'2"` (of course the password is not a real one :p) To reproduce: - use a password with % inside peertube_secret - upload to peertube only ``` python -m prismedia --file="path/files.mp4" --platform=peertube ``` You should not need a valid peertube access, as it breaks before upload.
rigelk commented 5 years ago

Ah, that's because I switched from RawConfigParser to ConfigParser… :X

I'll have to switch back...

Ah, that's because I switched from RawConfigParser to ConfigParser… :X I'll have to switch back...
rigelk commented 5 years ago

Rollback done. Could you please test?

Rollback done. Could you please test?
LecygneNoir commented 5 years ago

I have tested, I can confirm it now works! 👍

I have tested, I can confirm it now works! :+1:
prismedia/utils.py Outdated
# defined (None or False)
for key, value in viewitems(options):
key = key.replace("-", "")
print(key)
LecygneNoir commented 5 years ago

Forgot a print key here ;-)

Forgot a print key here ;-)
rigelk commented 5 years ago

ah. AH x)

ah. AH x)
LecygneNoir reviewed 5 years ago
for s in toclean:
if s == '':
continue
strtoclean = unicodedata.normalize('NFKD', unicode(s, 'utf-8')).encode('ASCII', 'ignore')
LecygneNoir commented 5 years ago

This line does not work with python3

Unfortunately, in python3 they have changed the unicode type that does not longer exist, in favor of str : https://docs.python.org/dev/howto/pyporting.html#text-versus-binary-data:

# python 3.7.2
2019-02-23 10:49:04,384 Peertube: Error: name 'unicode' is not defined

However, we cannot use str direclty in python2 as this is not the same:

#python 2.7.15
2019-02-23 10:58:52,130 Peertube: Error: str() takes at most 1 argument (2 given)

I am looking for a way to have something working in both case, I'll let you know if I found something, does not hesitate to tell if you have any idea!

This line does not work with python3 Unfortunately, in python3 they have changed the `unicode` type that does not longer exist, in favor of `str` : [https://docs.python.org/dev/howto/pyporting.html#text-versus-binary-data](https://docs.python.org/dev/howto/pyporting.html#text-versus-binary-data): ``` # python 3.7.2 2019-02-23 10:49:04,384 Peertube: Error: name 'unicode' is not defined ``` However, we cannot use `str` direclty in python2 as this is not the same: ``` #python 2.7.15 2019-02-23 10:58:52,130 Peertube: Error: str() takes at most 1 argument (2 given) ``` I am looking for a way to have something working in both case, I'll let you know if I found something, does not hesitate to tell if you have any idea!
LecygneNoir commented 5 years ago

To reproduce you does not need a working account on Peertube instance:

± python -m prismedia --file="path/file.mp4" --nfo nfo_example.txt
2019-02-23 10:48:53,012 Using nfo_example.txt as NFO, loading...
2019-02-23 10:49:04,142 Peertube: Uploading video...
2019-02-23 10:49:04,384 Peertube: Error: name 'unicode' is not defined
To reproduce you does not need a working account on Peertube instance: ``` ± python -m prismedia --file="path/file.mp4" --nfo nfo_example.txt 2019-02-23 10:48:53,012 Using nfo_example.txt as NFO, loading... 2019-02-23 10:49:04,142 Peertube: Uploading video... 2019-02-23 10:49:04,384 Peertube: Error: name 'unicode' is not defined ```
LecygneNoir commented 5 years ago

The simple way should be to do something like that:

        try:
            strtoclean = unicodedata.normalize('NFKD', unicode(s, 'utf-8')).encode('ASCII', 'ignore')
        except:
            strtoclean = unicodedata.normalize('NFKD', str(s, 'utf-8')).encode('ASCII', 'ignore')

But as we did more treatment than juste returning the string, this broke other things in the code :-/

Error: decoding str is not supported

Apparently it's the strtoclean = unicodedata.normalize('NFKD', str(s, 'utf-8')).encode('ASCII', 'ignore') that does not work, even if we never call to decode directly, all these method should call it implicitely, I do not find a workaround at the moment.

The simple way should be to do something like that: ``` try: strtoclean = unicodedata.normalize('NFKD', unicode(s, 'utf-8')).encode('ASCII', 'ignore') except: strtoclean = unicodedata.normalize('NFKD', str(s, 'utf-8')).encode('ASCII', 'ignore') ``` But as we did more treatment than juste returning the string, this broke other things in the code :-/ ``` Error: decoding str is not supported ``` Apparently it's the `strtoclean = unicodedata.normalize('NFKD', str(s, 'utf-8')).encode('ASCII', 'ignore')` that does not work, even if we never call to `decode` directly, all these method should call it implicitely, I do not find a workaround at the moment.
LecygneNoir commented 5 years ago

This point should now be easiest thanks to v0.6.1-1

I have rewritten this code to use less python2 specific code based on unicode/str differences between python2 and 3, with the help of the SIX library you use in other places in the code, we should be able make this part compatible python3!

This point should now be easiest thanks to v0.6.1-1 I have rewritten this code to use less python2 specific code based on unicode/str differences between python2 and 3, with the help of the `SIX` library you use in other places in the code, we should be able make this part compatible python3!
Zykino commented 5 years ago
Poster

Well I tried to use this branch on Windows and I couldn’t make it work neither on python2.7 nor 3.7. Also I’m not using python on a regular basis so I may have made something wrong.

>py -3 poetry install
(null): can't open file 'poetry': [Errno 2] No such file or directory

>py -3 -m prismedia
Traceback (most recent call last):
 File "C:\Program Files\Python37\lib\runpy.py", line 183, in _run_module_as_main
   mod_name, mod_spec, code = _get_module_details(mod_name, _Error)
 File "C:\Program Files\Python37\lib\runpy.py", line 142, in _get_module_details
   return _get_module_details(pkg_main_name, error)
 File "C:\Program Files\Python37\lib\runpy.py", line 109, in _get_module_details
   __import__(pkg_name)
 File "C:\Users\[PATH_TO_PRISMEDIA]\prismedia\__init__.py", line 4, in <module>
   from . import upload
 File "C:\Users\[PATH_TO_PRISMEDIA]\prismedia\upload.py", line 69, in <module>
   from . import yt_upload
 File "C:\Users\[PATH_TO_PRISMEDIA]\prismedia\yt_upload.py", line 24, in <module>
   from . import utils
 File "C:\Users\[PATH_TO_PRISMEDIA]\prismedia\utils.py", line 4, in <module>
   from ConfigParser import RawConfigParser, NoOptionError, NoSectionError
ModuleNotFoundError: No module named 'ConfigParser'
>poetry install
[no log but it worked]

>py -2 -m prismedia
Traceback (most recent call last):
 File "C:\Program Files\Python27\lib\runpy.py", line 163, in _run_module_as_main
   mod_name, _Error)
 File "C:\Program Files\Python27\lib\runpy.py", line 111, in _get_module_details
   __import__(mod_name)  # Do not catch exceptions initializing package
 File "prismedia\__init__.py", line 4, in <module>
   from . import upload
 File "prismedia\upload.py", line 69, in <module>
   from . import yt_upload
 File "prismedia\yt_upload.py", line 24, in <module>
   from . import utils
 File "prismedia\utils.py", line 9, in <module>
   import unidecode
ImportError: No module named unidecode
Well I tried to use this branch on Windows and I couldn’t make it work neither on python2.7 nor 3.7. Also I’m not using python on a regular basis so I may have made something wrong. ``` >py -3 poetry install (null): can't open file 'poetry': [Errno 2] No such file or directory >py -3 -m prismedia Traceback (most recent call last): File "C:\Program Files\Python37\lib\runpy.py", line 183, in _run_module_as_main mod_name, mod_spec, code = _get_module_details(mod_name, _Error) File "C:\Program Files\Python37\lib\runpy.py", line 142, in _get_module_details return _get_module_details(pkg_main_name, error) File "C:\Program Files\Python37\lib\runpy.py", line 109, in _get_module_details __import__(pkg_name) File "C:\Users\[PATH_TO_PRISMEDIA]\prismedia\__init__.py", line 4, in <module> from . import upload File "C:\Users\[PATH_TO_PRISMEDIA]\prismedia\upload.py", line 69, in <module> from . import yt_upload File "C:\Users\[PATH_TO_PRISMEDIA]\prismedia\yt_upload.py", line 24, in <module> from . import utils File "C:\Users\[PATH_TO_PRISMEDIA]\prismedia\utils.py", line 4, in <module> from ConfigParser import RawConfigParser, NoOptionError, NoSectionError ModuleNotFoundError: No module named 'ConfigParser' ``` ``` >poetry install [no log but it worked] >py -2 -m prismedia Traceback (most recent call last): File "C:\Program Files\Python27\lib\runpy.py", line 163, in _run_module_as_main mod_name, _Error) File "C:\Program Files\Python27\lib\runpy.py", line 111, in _get_module_details __import__(mod_name) # Do not catch exceptions initializing package File "prismedia\__init__.py", line 4, in <module> from . import upload File "prismedia\upload.py", line 69, in <module> from . import yt_upload File "prismedia\yt_upload.py", line 24, in <module> from . import utils File "prismedia\utils.py", line 9, in <module> import unidecode ImportError: No module named unidecode ```
Poster
Owner

@zykino thank you for the test!

For the python3 test at the moment it does not work so... I am not surprised :'(

About python2 it seems you miss the unidecode module, indeed I have added some more prerequisites but I am not sure to have noted them somewhere

@zykino thank you for the test! For the python3 test at the moment it does not work so... I am not surprised :'( About python2 it seems you miss the unidecode module, indeed I have added some more prerequisites but I am not sure to have noted them somewhere
Poster
Owner

Hello @rigelk !

Finally as we discussed, now that prismedia uses python3 exclusively, I have been able to integrate poetry :-)

This MR should be solved by this commit: #cb39eef8e0a5e1b4a022c9ada6a6f2e64c5c5d5b

I have mainly take inspiration from your code, thank you a lot!

I'll probably publish on Pypi once the release 0.9.0 is done (I wan to add some more features)

Please feel free to review and tell me if there is anything wrong

I'll close this MR now it's obsolete.

Thanks!

Hello @rigelk ! Finally as we discussed, now that prismedia uses python3 exclusively, I have been able to integrate poetry :-) This MR should be solved by this commit: #cb39eef8e0a5e1b4a022c9ada6a6f2e64c5c5d5b I have mainly take inspiration from your code, thank you a lot! I'll probably publish on Pypi once the release 0.9.0 is done (I wan to add some more features) Please feel free to review and tell me if there is anything wrong I'll close this MR now it's obsolete. Thanks!
LecygneNoir closed this pull request 4 years ago
Please reopen this pull request to perform a merge.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.