Move repl to a seperate repo

dhoomakethu commented 7 months ago

REPL code moved here https://github.com/pymodbus-dev/repl
pymodbus.console and pymodbus.server are still available once installed as before
Installation of optional repl feature is still the same pip install ".[repl]"

This way we can keep pymodbus core library separate from all the helpers and utils.

TODO

Tests and code checks on repl repo to be used
console and server commands use different approach for commands and hints, to be moved to one common lib.

NOTE

This is just an idea, we can discuss the best coure of action and take action with this PR accordingly,

janiversen commented 7 months ago

Seems at least one test is also affected

ImportError while importing test module '/home/runner/work/pymodbus/pymodbus/test/sub_client/test_repl_client.py'.

janiversen commented 7 months ago

I think moving pymodbus.server/client to the new repo is better than having a third directory, but I am open to ideas.

alexrudd2 commented 7 months ago

Interesting idea.

Would it make more sense for REPL to be a separate package that depends on pymodbus? That way it could stay on a stable version until newer ones have been tested.

dhoomakethu commented 7 months ago

I think moving pymodbus.server/client to the new repo is better than having a third directory, but I am open to ideas.

@janiversen are you suggesting move the entire server and client to a new repo or are you talking about the REPL server and client ?

dhoomakethu commented 7 months ago

Interesting idea.

Would it make more sense for REPL to be a separate package that depends on pymodbus? That way it could stay on a stable version until newer ones have been tested.

The idea would be to have it installed as stand-alone package and also as part of pymodbus library on demand, we can see if it makes sense to have it as a seperate repo instead of a subpackage to pymodbus.

janiversen commented 7 months ago

Would it make more sense for REPL to be a separate package that depends on pymodbus? That way it could stay on a stable version until newer ones have been tested.

Having it as a new repo that depends on a specific version of pymodbus does exactly that.

@dhoomakethu my idea was to move REPL client/server packages to a separate repo, that depends on pymodbus. That way updates to pymodbus does not affect the REPL packages, and REPL can have it own release schedule.

The idea would be to have it installed as stand-alone package and also as part of pymodbus library on demand, we can see if it makes sense to have it as a seperate repo instead of a subpackage to pymodbus.

The focus is to make the REPL package resistent to changes in pymodbus !! I am not sure how you can do that if it is one repo. However I do not have a lot of experience in this, but I see the following problems:

Github have only 1 release tree for each repo
REPL and pymodbus would at least share pyproject.toml, .github, readme etc...
REPL would also always use the newest pymodbus, so it would not be resistent to pymodbus changes So in short, if I have understood sub-package correctly in terms of GitHub, I do not see the big difference between having a seperate repo or a sub-package

Remark: if you mean subproject, then that is a separate repo, just present in the pymodbus tree. But that would mean all pymodbus changes would affect REPL. If a new REPL repo have pymodbus as a subproject, it decides when to use a new pymodbus version.

janiversen commented 7 months ago

As far as I can judge, this PR remove REPL client/server from the pymodbus repo...so please explain how you where/how you would add it, because it really seems we are thinking in different terms (and my thinking might very well be wrong).

dhoomakethu commented 7 months ago

As far as I can judge, this PR remove REPL client/server from the pymodbus repo...so please explain how you where/how you would add it, because it really seems we are thinking in different terms (and my thinking might very well be wrong).

@janiversen If you check the pyproject.toml in the PR, when somebody uses pip install pymodbus[repl] or pip install ".[repl]" or pip install ".[all]", PIP will pull the source from https://github.com/pymodbus-dev/repl repo and install it in the pymodbus tree

repl = [
    "aiohttp>=3.8.6;python_version<'3.12'",
    "aiohttp>=3.9.0b0;python_version=='3.12'",
    "pymodbus-repl@git+https://github.com/pymodbus-dev/repl"
]

pymodbus is a dev dependency in repl project (seperate repo) and the folder structure follows packing subpackages (repl) for a namespace package (in this case pymodbus) .

For more info on namespace packages refer. https://packaging.python.org/en/latest/guides/packaging-namespace-packages/

Now, the good thing about this is

we are moving the repl source code away from the core pymodbus library and hence its one head-ache less for people like you who are actively maintaining the core project.
repl can now be installed as a subpackage to pymodbus when required from a different repo.
repl can also be installed as a seperate package altogether with pymodbus as a dependency. Right now it requires the latest and greatest version of pymodbus.

For 3. above, There is no seperate PIP package available as of now and it needs to be installed from github directly. We can think of creating the distro depending on the demand.

dhoomakethu commented 7 months ago

Would it make more sense for REPL to be a separate package that depends on pymodbus? That way it could stay on a stable version until newer ones have been tested.

Having it as a new repo that depends on a specific version of pymodbus does exactly that.

@dhoomakethu my idea was to move REPL client/server packages to a separate repo, that depends on pymodbus. That way updates to pymodbus does not affect the REPL packages, and REPL can have it own release schedule.

The idea would be to have it installed as stand-alone package and also as part of pymodbus library on demand, we can see if it makes sense to have it as a seperate repo instead of a subpackage to pymodbus.

The focus is to make the REPL package resistent to changes in pymodbus !! I am not sure how you can do that if it is one repo. However I do not have a lot of experience in this, but I see the following problems:

Github have only 1 release tree for each repo

REPL and pymodbus would at least share pyproject.toml, .github, readme etc...

REPL would also always use the newest pymodbus, so it would not be resistent to pymodbus changes So in short, if I have understood sub-package correctly in terms of GitHub, I do not see the big difference between having a seperate repo or a sub-package

Remark: if you mean subproject, then that is a separate repo, just present in the pymodbus tree. But that would mean all pymodbus changes would affect REPL. If a new REPL repo have pymodbus as a subproject, it decides when to use a new pymodbus version.

Please refer the comment above https://github.com/pymodbus-dev/pymodbus/pull/2009#issuecomment-1942948549

alexrudd2 commented 7 months ago

side note: With Chicago, Grenada and Bengaluru we have split the world timezones pretty well so that something is always happening in pymodbus :)

janiversen commented 7 months ago

@dhoomakethu thanks for the explanation, the missing piece for me was py project.toml, because I was merely thinking about github.

I will check it in a minute.

janiversen commented 7 months ago

Ahh the culprit is documentation, you need to remove repl from there.

dhoomakethu commented 7 months ago

Ahh the culprit is documentation, you need to remove repl from there.

@janiversen The CI failure is happening for python 3.12 and I see the documentation steps are skipped in other versions. Is this something known ?

janiversen commented 7 months ago

Yes, we only build documentation for the newest python version, mainly because the documentation have no dependencies on python versions, so doing it for all versions will just make CI slower.

Thanks for noticing.

dhoomakethu commented 7 months ago

Yes, we only build documentation for the newest python version, mainly because the documentation have no dependencies on python versions, so doing it for all versions will just make CI slower.

Thanks for noticing.

@janiversen so is it OK if the documentation is failing ? The error looks like a generic PYTHONPATH resolution issue and not having any thing to do with the changes introduced in this branch.

janiversen commented 7 months ago

No it's not OK, I think you need to remove a document and a reference....

doc/index.rst
doc/source/REPL.rst
doc/source/simulator.rst

index.rst contains a reference to REPL.rst REPL.rst should be removed simulator.rst contains a reference to REPL

The problem is the REPL.rst have a reference into pymodbus/repl, which is removed.

dhoomakethu commented 7 months ago

The CI is failing with this error for documentation

Running Sphinx v7.2.6

Configuration error:
There is a programmable error in your configuration file:

Traceback (most recent call last):
  File "/home/runner/work/pymodbus/pymodbus/venv/lib/python3.12/site-packages/sphinx/config.py", line 358, in eval_config_file
    exec(code, namespace)  # NoQA: S102
    ^^^^^^^^^^^^^^^^^^^^^
  File "/home/runner/work/pymodbus/pymodbus/doc/conf.py", line 14, in <module>
    from pymodbus import __version__ as pymodbus_version
ImportError: cannot import name '__version__' from 'pymodbus' (unknown location)

However trying to build locally works

der in 🌐 pop-os in pymodbus/doc on  repl-as-plugin [⇡] via 🐍 v3.10.12 (pymodbus) took 12s
❯ ./build_html
~/repo/pymodbus ~/repo/pymodbus/doc
updating: examples/__init__.py (stored 0%)
updating: examples/certificates/ (stored 0%)
updating: examples/client_async.py (deflated 69%)
updating: examples/client_async_calls.py (deflated 76%)
updating: examples/client_calls.py (deflated 74%)
updating: examples/client_custom_msg.py (deflated 70%)
updating: examples/client_payload.py (deflated 71%)
updating: examples/client_performance.py (deflated 71%)
updating: examples/client_sync.py (deflated 70%)
updating: examples/contrib/ (stored 0%)
updating: examples/helper.py (deflated 67%)
updating: examples/message_generator.py (deflated 78%)
updating: examples/message_parser.py (deflated 70%)
updating: examples/modbus_forwarder.py (deflated 59%)
updating: examples/server_async.py (deflated 73%)
updating: examples/server_callback.py (deflated 62%)
updating: examples/server_hook.py (deflated 62%)
updating: examples/server_payload.py (deflated 66%)
updating: examples/server_sync.py (deflated 72%)
updating: examples/server_updating.py (deflated 62%)
updating: examples/simple_async_client.py (deflated 67%)
updating: examples/simple_sync_client.py (deflated 68%)
updating: examples/simulator.py (deflated 65%)
updating: examples/datastore_simulator_share.py (deflated 67%)
~/repo/pymodbus/doc
Running Sphinx v7.2.6
loading pickled environment... done
building [mo]: targets for 0 po files that are out of date
writing output...
building [html]: targets for 0 source files that are out of date
updating environment: [config changed ('recommonmark_config')] 21 added, 0 changed, 0 removed
reading sources... [100%] source/simulator
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
copying downloadable files... [100%] source/_static/examples.tgz
copying static files... done
copying extra files... done
done
writing output... [100%] source/simulator
generating indices... genindex py-modindex done
writing additional pages... search done
copying images... [100%] source/library/simulator/modbuscommonblock.png
dumping search index in English (code: en)... done
dumping object inventory... done
build succeeded.

The HTML pages are in ../build/html/html.

Will check if its specific to python3.12

dhoomakethu commented 7 months ago

works on python3.12-.2 locally

rider in 🌐 pop-os in pymodbus/doc on  repl-as-plugin [!] via 🐍 v3.12.2+ (pymodbus3.12)
❯ ./build_html
~/repo/pymodbus ~/repo/pymodbus/doc
updating: examples/__init__.py (stored 0%)
updating: examples/certificates/ (stored 0%)
updating: examples/client_async.py (deflated 69%)
updating: examples/client_async_calls.py (deflated 76%)
updating: examples/client_calls.py (deflated 74%)
updating: examples/client_custom_msg.py (deflated 70%)
updating: examples/client_payload.py (deflated 71%)
updating: examples/client_performance.py (deflated 71%)
updating: examples/client_sync.py (deflated 70%)
updating: examples/contrib/ (stored 0%)
updating: examples/helper.py (deflated 67%)
updating: examples/message_generator.py (deflated 78%)
updating: examples/message_parser.py (deflated 70%)
updating: examples/modbus_forwarder.py (deflated 59%)
updating: examples/server_async.py (deflated 73%)
updating: examples/server_callback.py (deflated 62%)
updating: examples/server_hook.py (deflated 62%)
updating: examples/server_payload.py (deflated 66%)
updating: examples/server_sync.py (deflated 72%)
updating: examples/server_updating.py (deflated 62%)
updating: examples/simple_async_client.py (deflated 67%)
updating: examples/simple_sync_client.py (deflated 68%)
updating: examples/simulator.py (deflated 65%)
updating: examples/datastore_simulator_share.py (deflated 67%)
~/repo/pymodbus/doc
Running Sphinx v7.2.6
loading pickled environment... done
building [mo]: targets for 0 po files that are out of date
writing output...
building [html]: targets for 21 source files that are out of date
updating environment: [config changed ('recommonmark_config')] 21 added, 0 changed, 0 removed
reading sources... [100%] source/simulator
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
copying downloadable files... [100%] source/_static/examples.tgz
copying static files... done
copying extra files... done
done
writing output... [100%] source/simulator
generating indices... genindex py-modindex done
writing additional pages... search done
copying images... [100%] source/library/simulator/modbuscommonblock.png
dumping search index in English (code: en)... done
dumping object inventory... done
build succeeded.

The HTML pages are in ../build/html/html.

janiversen commented 7 months ago

but it works normally as you can see on dev.

I will try to checkout your PR and see what happens.

janiversen commented 7 months ago

This is real strange, dev works perfectly on CI but this PR do not....something does something unexpected.

dhoomakethu commented 7 months ago

I will try to poke around a bit. Will try to reproduce this locally, my wild guess is this is something to do with cached env's.

dhoomakethu commented 7 months ago

I could reproduce the issue, the issue was with the CI trying to install pymodbus in editable mode with -e flag. This was causing the subpackage installation of pymodbus-repl overwrite the entire pymodbus directory in the site-package with only the content of repl repo.

- name: Create venv (NEW CACHE)
        if: steps.cache-venv.outputs.cache-hit != 'true'
        run: |
          python -m venv ${{ env.VIRTUAL_ENV }}
          python -m pip install --upgrade pip
          pip install -e ".[all]"

With `pip install -e ".[all]"

pymodbus/doc on  repl-as-plugin [$✘!?] via 🐍 v3.12.0b3 (pymodbus3.12)
❯ ls -la .pyenv/versions/3.12.0b3/envs/pymodbus3.12/lib/python3.12/site-packages/pymodbus
total 0
drwxr-xr-x    3 sanjay  staff    96 Feb 14 19:53 .
drwxr-xr-x  177 sanjay  staff  5664 Feb 14 19:53 ..
drwxr-xr-x    7 sanjay  staff   224 Feb 14 19:53 repl

With `pip install ".[all]"

pymodbus on  repl-as-plugin [$✘!?] via 🐍 v3.12.0b3 (pymodbus3.12) took 16s
❯ ls -la .pyenv/versions/3.12.0b3/envs/pymodbus3.12/lib/python3.12/site-packages/pymodbus
total 496
drwxr-xr-x   29 sanjay  staff    928 Feb 14 19:54 .
drwxr-xr-x  175 sanjay  staff   5600 Feb 14 19:54 ..
-rw-r--r--    1 sanjay  staff    507 Feb 14 19:54 __init__.py
drwxr-xr-x   21 sanjay  staff    672 Feb 14 19:54 __pycache__
-rw-r--r--    1 sanjay  staff   9422 Feb 14 19:54 bit_read_message.py
-rw-r--r--    1 sanjay  staff   9536 Feb 14 19:54 bit_write_message.py
drwxr-xr-x   10 sanjay  staff    320 Feb 14 19:54 client
-rw-r--r--    1 sanjay  staff   3499 Feb 14 19:54 constants.py
drwxr-xr-x    8 sanjay  staff    256 Feb 14 19:54 datastore
-rw-r--r--    1 sanjay  staff  22601 Feb 14 19:54 device.py
-rw-r--r--    1 sanjay  staff  30458 Feb 14 19:54 diag_message.py
-rw-r--r--    1 sanjay  staff   6482 Feb 14 19:54 events.py
-rw-r--r--    1 sanjay  staff   3189 Feb 14 19:54 exceptions.py
-rw-r--r--    1 sanjay  staff  13656 Feb 14 19:54 factory.py
-rw-r--r--    1 sanjay  staff  14708 Feb 14 19:54 file_message.py
drwxr-xr-x   10 sanjay  staff    320 Feb 14 19:54 framer
-rw-r--r--    1 sanjay  staff   3926 Feb 14 19:54 logging.py
-rw-r--r--    1 sanjay  staff   7847 Feb 14 19:54 mei_message.py
-rw-r--r--    1 sanjay  staff  15363 Feb 14 19:54 other_message.py
-rw-r--r--    1 sanjay  staff  15663 Feb 14 19:54 payload.py
-rw-r--r--    1 sanjay  staff   7717 Feb 14 19:54 pdu.py
-rw-r--r--    1 sanjay  staff      0 Feb 14 19:54 py.typed
-rw-r--r--    1 sanjay  staff  14208 Feb 14 19:54 register_read_message.py
-rw-r--r--    1 sanjay  staff  12510 Feb 14 19:54 register_write_message.py
drwxr-xr-x    7 sanjay  staff    224 Feb 14 19:53 repl
drwxr-xr-x    7 sanjay  staff    224 Feb 14 19:54 server
-rw-r--r--    1 sanjay  staff  21789 Feb 14 19:54 transaction.py
drwxr-xr-x    6 sanjay  staff    192 Feb 14 19:54 transport
-rw-r--r--    1 sanjay  staff   7697 Feb 14 19:54 utilities.py
(pymodbus3.12)

janiversen commented 7 months ago

good catch, I did not think of that.

I just removed the aiohttp dependencies of REPL, those should be in the repl repo.

janiversen commented 7 months ago

If this one get green, then lets merge.

janiversen commented 7 months ago

This PR was one of the heavier ones, but thanks for helping making it happen.

@dhoomakethu having REPL in a separate repo, means it is primarily your child....but poke me if you need help etc ...I do not want to abandon REPL, which is a very good tool.

If you want the REPL documentation to be part of the pymodbus documentation (but with its own versioning), just know I am all in favor !

dhoomakethu commented 7 months ago

We will have the documentation moved to repl repo and link it in pymodbus. I will update the PR tomorrow

janiversen commented 7 months ago

OK, looking forward to see that PR.

pymodbus-dev / pymodbus

Move repl to a seperate repo #2009

TODO