Conversation
4b62446 to
d1d2505
Compare
d1d2505 to
1c58681
Compare
andreoliwa
left a comment
There was a problem hiding this comment.
Let's talk about class names (or not), and I couldn't make tox work (probably I'm doing something wwrong or missing).
docs/usage.rst
Outdated
|
|
||
| fsm_class = db.Column(db.String) | ||
| model_class = db.Column(db.String) | ||
| current_state= = db.Column(db.String) |
docs/usage.rst
Outdated
| Using events with enums instead of simple strings | ||
| ================================================= | ||
|
|
||
| In case you want to have a sane event naming, probably it is better to use constants or python's enum module. |
There was a problem hiding this comment.
"Python" with uppercase "P".
docs/usage.rst
Outdated
| refunded = properties.FinalState() | ||
| charged_back = properties.FinalState() | ||
|
|
||
| The state machine will be validated when it gets parsed by python interpreter and below you can find a visual |
There was a problem hiding this comment.
"Python" with uppercase "P".
| self.current_state_instance.timeout.timedelta)) | ||
| db.session.add(timeout) | ||
|
|
||
| Using events with enums instead of simple strings |
There was a problem hiding this comment.
Nice example. It works for future code, and we don't need to change our Cave code right now. 👏
src/tuco/base.py
Outdated
|
|
||
| import pytz | ||
|
|
||
| from tuco.exceptions import TucoAlreadyLocked, TucoEventNotFound, TucoInvalidStateChange, TucoInvalidStateHolder |
There was a problem hiding this comment.
Should we use the suffix Error for all exceptions (AlreadyLockedError, EventNotFoundError...)?
Is this a Python convention?
I read this once, but I couldn't find the reference now.
Or just replace Tuco by FSM: FSMAlreadyLocked, FSMEventNotFound...
Just because the base class is called FSM (because it makes more sense, I agree), not Tuco.
There was a problem hiding this comment.
They were all prefixed with FSM but I was afraid of it conflicting with other libraries in the future.
There was a problem hiding this comment.
The names could conflict if a project uses Tuco and another FSM module at the same time, which is unlikely.
But the Tuco prefix is not a big issue, it's just that I still don't like the project name.
I think the Error suffix is more important.
We should stick to Python conventions if we want our module to be used by devs.
Although I'm not sure if using the Error suffix is actually a convention.
There was a problem hiding this comment.
If you are switching between state machines you could potentially have the two at the same time. I will also change the suffix
There was a problem hiding this comment.
Why would someone switch to another FSM module? 😄
There was a problem hiding this comment.
idk, if they found a better one, why not?
src/tuco/base.py
Outdated
| return self | ||
|
|
||
| def __exit__(self, exc_type, exc_val, exc_tb): | ||
| """If FSMAlreadyLocked did not throw, unlock the machine.""" |
There was a problem hiding this comment.
Class name doesn't match the code, it should be TucoAlreadyLocked (depending on what's decided from the comment above).
src/tuco/generate_graph.py
Outdated
| from tuco.properties import Error, Event, FinalState, State, Timeout | ||
|
|
||
|
|
||
| class GenerateGraph: |
There was a problem hiding this comment.
-
Classes should be nouns, something like
GraphBuilder.
I would not useGraphGeneratorto not mix it with Python generators. -
I would also rename the module file from
generate_graph.pytograph_builder.py(or whatever noun is used).
tox.ini
Outdated
| docutils | ||
| check-manifest | ||
| flake8 | ||
| flake8-mypy |
There was a problem hiding this comment.
We could add all the plugins we use for other projects, no?
Those:
flake8-blind-except = "*"
flake8-bugbear = "*"
flake8-comprehensions = "*"
flake8-debugger = "*"
flake8-docstrings = "*"
flake8-isort = "*"
flake8-mypy = "*"
flake8-polyfill = "*"
flake8-pytest = "*"
flake8-quotes = "*"
There was a problem hiding this comment.
(after our Slack discussion)
Well, we could remove most of those actually.
They are “purist” plugins
We already have mypy, isort, and flake8 itself, they are good enough.
Maybe keep the flake8-quotes, for quote consistency?
My point is: when I see a project that has standardised code, with lots of comments, it looks reliable.
We could remove flake8-docstrings, which is picky and adds a lot of noise comments.
We should just keep good documentation by ourselves, without any flake8 police.
So the new list could be:
flake8-isort
flake8-mypy
flake8-quotes
Thoughts?
There was a problem hiding this comment.
All, good. Changing it on my side.
| To run the all tests run:: | ||
| Make sure you have a running redis instance and to run the all tests run:: | ||
|
|
||
| tox |
There was a problem hiding this comment.
I tried to clone it locally to run tests:
git clone [email protected]:eatfirst/python-tuco.git- Set up a virtual environment with pyenv (and
dotfiles). pip install toxtox
And the result was:
$ tox
GLOB sdist-make: /Users/wagneraugusto/eatfirst/python-tuco/setup.py
clean create: /Users/wagneraugusto/eatfirst/python-tuco/.tox/clean
clean installdeps: coverage
clean installed: coverage==4.4.2
clean runtests: PYTHONHASHSEED='1402629706'
clean runtests: commands[0] | coverage erase
check create: /Users/wagneraugusto/eatfirst/python-tuco/.tox/check
check installdeps: docutils, check-manifest, flake8, flake8-mypy, readme-renderer, pygments, isort
check installed: attrs==17.4.0,bleach==2.1.2,check-manifest==0.36,docutils==0.14,flake8==3.5.0,flake8-mypy==17.8.0,html5lib==1.0.1,isort==4.2.15,mccabe==0.6.1,mypy==0.560,psutil==5.4.3,pycodestyle==2.3.1,pyflakes==1.6.0,Pygments==2.2.0,readme-renderer==17.2,six==1.11.0,typed-ast==1.1.0,webencodings==0.5.1
check runtests: PYTHONHASHSEED='1402629706'
check runtests: commands[0] | python setup.py check --strict --metadata --restructuredtext
running check
check runtests: commands[1] | check-manifest /Users/wagneraugusto/eatfirst/python-tuco
lists of files in version control and sdist match
check runtests: commands[2] | flake8 src tests setup.py
check runtests: commands[3] | isort --verbose --check-only --diff --recursive src tests setup.py
/#######################################################################\
`sMMy`
.yyyy- `
##soos## ./o.
` ``..-..` ``...`.`` ` ```` ``-ssso```
.s:-y- .+osssssso/. ./ossss+:so+:` :+o-`/osso:+sssssssso/
.s::y- osss+.``.`` -ssss+-.`-ossso` ssssso/::..::+ssss:::.
.s::y- /ssss+//:-.` `ssss+ `ssss+ sssso` :ssss`
.s::y- `-/+oossssso/ `ssss/ sssso ssss/ :ssss`
.y-/y- ````:ssss` ossso. :ssss: ssss/ :ssss.
`/so:` `-//::/osss+ `+ssss+-/ossso: /sso- `osssso/.
\/ `-/oooo++/- .:/++:/++/-` .. `://++/.
isort your Python imports for you so you don't have to
VERSION 4.2.15
\########################################################################/
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/generate_graph.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/properties.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/__init__.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/exceptions.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/base.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/meta.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/decorators.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/locks/memory.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/locks/__init__.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/locks/redis.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/locks/base.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/tests/test_tuco.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/tests/example_fsm.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/tests/__init__.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/tests/test_graph_creation.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/setup.py Everything Looks Good!
py34 create: /Users/wagneraugusto/eatfirst/python-tuco/.tox/py34
ERROR: InterpreterNotFound: python3.4
py35 create: /Users/wagneraugusto/eatfirst/python-tuco/.tox/py35
ERROR: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.5: b"pyenv: python3.5: command not found\\n\\nThe `python3.5\' command exists in these Python versions:\\n 3.5.4\\n\\n"',)
py36 create: /Users/wagneraugusto/eatfirst/python-tuco/.tox/py36
ERROR: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.6: b"pyenv: python3.6: command not found\\n\\nThe `python3.6\' command exists in these Python versions:\\n 3.6.2\\n 3.6.3\\n\\n"',)
pypy create: /Users/wagneraugusto/eatfirst/python-tuco/.tox/pypy
ERROR: InterpreterNotFound: pypy
report create: /Users/wagneraugusto/eatfirst/python-tuco/.tox/report
report installdeps: coverage
report installed: coverage==4.4.2
report runtests: PYTHONHASHSEED='1402629706'
report runtests: commands[0] | coverage combine --append
No data to combine
ERROR: InvocationError: '/Users/wagneraugusto/eatfirst/python-tuco/.tox/report/bin/coverage combine --append'
docs create: /Users/wagneraugusto/eatfirst/python-tuco/.tox/docs
ERROR: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.6: b"pyenv: python3.6: command not found\\n\\nThe `python3.6\' command exists in these Python versions:\\n 3.6.2\\n 3.6.3\\n\\n"',)
_________________________________________________________________________________________________ summary __________________________________________________________________________________________________
clean: commands succeeded
check: commands succeeded
ERROR: py34: InterpreterNotFound: python3.4
ERROR: py35: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.5: b"pyenv: python3.5: command not found\\n\\nThe `python3.5\' command exists in these Python versions:\\n 3.5.4\\n\\n"',)
ERROR: py36: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.6: b"pyenv: python3.6: command not found\\n\\nThe `python3.6\' command exists in these Python versions:\\n 3.6.2\\n 3.6.3\\n\\n"',)
ERROR: pypy: InterpreterNotFound: pypy
ERROR: report: commands failed
ERROR: docs: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.6: b"pyenv: python3.6: command not found\\n\\nThe `python3.6\' command exists in these Python versions:\\n 3.6.2\\n 3.6.3\\n\\n"',)
There was a problem hiding this comment.
You need to have all this python versions in your path if you want to run tests in all version
There was a problem hiding this comment.
I installed Python 3.4.7, after setting some compilation flags on MacOS:
CFLAGS="-I$(brew --prefix openssl)/include -I$(xcrun --show-sdk-path)/usr/include" \
LDFLAGS="-L$(brew --prefix openssl)/lib" \
pyenv install -v 3.4.7
But still no success with tox:
ERROR: py34: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.4: b"pyenv: python3.4: command not found\\n\\nThe `python3.4\' command exists in these Python versions:\\n 3.4.7\\n\\n"',)
ERROR: py35: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.5: b"pyenv: python3.5: command not found\\n\\nThe `python3.5\' command exists in these Python versions:\\n 3.5.4\\n\\n"',)
ERROR: py36: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.6: b"pyenv: python3.6: command not found\\n\\nThe `python3.6\' command exists in these Python versions:\\n 3.6.2\\n 3.6.3\\n\\n"',)
ERROR: report: commands failed
ERROR: docs: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.6: b"pyenv: python3.6: command not found\\n\\nThe `python3.6\' command exists in these Python versions:\\n 3.6.2\\n 3.6.3\\n\\n"',)
Full log:
$ tox
GLOB sdist-make: /Users/wagneraugusto/eatfirst/python-tuco/setup.py
clean installed: coverage==4.4.2
clean runtests: PYTHONHASHSEED='2128799634'
clean runtests: commands[0] | coverage erase
check installed: bleach==2.1.2,check-manifest==0.36,docutils==0.14,flake8==3.5.0,flake8-quotes==0.13.0,html5lib==1.0.1,isort==4.2.15,mccabe==0.6.1,mypy==0.560,psutil==5.4.3,pycodestyle==2.3.1,pyflakes==1.6.0,Pygments==2.2.0,readme-renderer==17.2,six==1.11.0,typed-ast==1.1.0,webencodings==0.5.1
check runtests: PYTHONHASHSEED='2128799634'
check runtests: commands[0] | python setup.py check --strict --metadata --restructuredtext
running check
check runtests: commands[1] | check-manifest /Users/wagneraugusto/eatfirst/python-tuco
lists of files in version control and sdist match
check runtests: commands[2] | flake8 src tests setup.py
check runtests: commands[3] | isort --verbose --check-only --diff --recursive src tests setup.py
/#######################################################################\
`sMMy`
.yyyy- `
##soos## ./o.
` ``..-..` ``...`.`` ` ```` ``-ssso```
.s:-y- .+osssssso/. ./ossss+:so+:` :+o-`/osso:+sssssssso/
.s::y- osss+.``.`` -ssss+-.`-ossso` ssssso/::..::+ssss:::.
.s::y- /ssss+//:-.` `ssss+ `ssss+ sssso` :ssss`
.s::y- `-/+oossssso/ `ssss/ sssso ssss/ :ssss`
.y-/y- ````:ssss` ossso. :ssss: ssss/ :ssss.
`/so:` `-//::/osss+ `+ssss+-/ossso: /sso- `osssso/.
\/ `-/oooo++/- .:/++:/++/-` .. `://++/.
isort your Python imports for you so you don't have to
VERSION 4.2.15
\########################################################################/
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/properties.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/__init__.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/exceptions.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/graph_builder.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/base.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/meta.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/decorators.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/locks/memory.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/locks/__init__.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/locks/redis.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/src/tuco/locks/base.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/tests/test_tuco.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/tests/example_fsm.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/tests/__init__.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/tests/test_graph_creation.py Everything Looks Good!
SUCCESS: /Users/wagneraugusto/eatfirst/python-tuco/setup.py Everything Looks Good!
check runtests: commands[4] | mypy src tests setup.py
py34 create: /Users/wagneraugusto/eatfirst/python-tuco/.tox/py34
ERROR: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.4: b"pyenv: python3.4: command not found\\n\\nThe `python3.4\' command exists in these Python versions:\\n 3.4.7\\n\\n"',)
py35 create: /Users/wagneraugusto/eatfirst/python-tuco/.tox/py35
ERROR: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.5: b"pyenv: python3.5: command not found\\n\\nThe `python3.5\' command exists in these Python versions:\\n 3.5.4\\n\\n"',)
py36 create: /Users/wagneraugusto/eatfirst/python-tuco/.tox/py36
ERROR: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.6: b"pyenv: python3.6: command not found\\n\\nThe `python3.6\' command exists in these Python versions:\\n 3.6.2\\n 3.6.3\\n\\n"',)
report installed: coverage==4.4.2
report runtests: PYTHONHASHSEED='2128799634'
report runtests: commands[0] | coverage combine --append
No data to combine
ERROR: InvocationError: '/Users/wagneraugusto/eatfirst/python-tuco/.tox/report/bin/coverage combine --append'
docs create: /Users/wagneraugusto/eatfirst/python-tuco/.tox/docs
ERROR: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.6: b"pyenv: python3.6: command not found\\n\\nThe `python3.6\' command exists in these Python versions:\\n 3.6.2\\n 3.6.3\\n\\n"',)
_________________________________________________________________________________________________ summary __________________________________________________________________________________________________
clean: commands succeeded
check: commands succeeded
ERROR: py34: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.4: b"pyenv: python3.4: command not found\\n\\nThe `python3.4\' command exists in these Python versions:\\n 3.4.7\\n\\n"',)
ERROR: py35: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.5: b"pyenv: python3.5: command not found\\n\\nThe `python3.5\' command exists in these Python versions:\\n 3.5.4\\n\\n"',)
ERROR: py36: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.6: b"pyenv: python3.6: command not found\\n\\nThe `python3.6\' command exists in these Python versions:\\n 3.6.2\\n 3.6.3\\n\\n"',)
ERROR: report: commands failed
ERROR: docs: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.6: b"pyenv: python3.6: command not found\\n\\nThe `python3.6\' command exists in these Python versions:\\n 3.6.2\\n 3.6.3\\n\\n"',)
I will investigate it later.
There was a problem hiding this comment.
as you are using pyenv it changes the paths so you have to run this before pyenv local 3.4.7 3.6.2 3.5.4
There was a problem hiding this comment.
At the end of the message it shows the pyenv output
ERROR: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError('Failed to get version_info for python3.6: b"pyenv: python3.6: command not found\n\nThe `python3.6' command exists in these Python versions:\n 3.6.2\n 3.6.3\n\n"',)
There was a problem hiding this comment.
so you have to run this before pyenv local 3.4.7 3.6.2 3.5.4
I don't know how to do that, never mind.
It's working, so all good.
I will read more about tox in the future.
And people that want to fork Tuco will probably know how to use tox.
andreoliwa
left a comment
There was a problem hiding this comment.
Approve it, so we can make the repo public.
The changes will be discussed and fixed later.
|
Changes Unknown when pulling 34dcb87 on initial-library-data into ** on master**. |
|
Changes Unknown when pulling 613d8d7 on initial-library-data into ** on master**. |
|
Changes Unknown when pulling 1c05216 on initial-library-data into ** on master**. |
1 similar comment
|
Changes Unknown when pulling 1c05216 on initial-library-data into ** on master**. |
|
Changes Unknown when pulling c8e4875 on initial-library-data into ** on master**. |
It is a version with support of python 2 and also we should target cPython at least for now
|
Changes Unknown when pulling 56f655d on initial-library-data into ** on master**. |
1 similar comment
|
Changes Unknown when pulling 56f655d on initial-library-data into ** on master**. |
|
Travis working but the job docs will be broken until we release the frist version https://travis-ci.org/eatfirst/python-tuco/jobs/325114115 |
| env: | ||
| - TOXENV=py36,report,coveralls | ||
| - python: 'pypy-5.4' | ||
| - python: '3.6' |
There was a problem hiding this comment.
Why 3.6 twice (lines 18 and 21)?
There was a problem hiding this comment.
This is a matrix, so it is python 3.6 and env TOXENV=docs so it has a dedicated docs tasks
There was a problem hiding this comment.
Hmm... so using - TOXENV=py36,report,coveralls,docs generates a different output?
I'm trying to understand how it works.
There was a problem hiding this comment.
No, it will create one TOXENV=py32,report,coveralls and one with TOXENV=docs both running with travis python3.6 support
|
Changes Unknown when pulling 419e671 on initial-library-data into ** on master**. |
No description provided.