Move repl to a seperate repo #2009

dhoomakethu · 2024-02-13T14:31:10Z

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

I think you need to update doc and the README as well.

pyproject.toml

janiversen · 2024-02-13T15:07:41Z

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 · 2024-02-13T15:10:19Z

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

alexrudd2 · 2024-02-13T15:44:01Z

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 · 2024-02-13T16:11:55Z

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 · 2024-02-13T16:13:32Z

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 · 2024-02-13T18:09:20Z

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 · 2024-02-13T18:12:09Z

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 · 2024-02-14T01:28:48Z

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 · 2024-02-14T01:29:57Z

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 #2009 (comment)

alexrudd2

I do not fully understand git submodules, but think @dhoomakethu has come up with a plan that considers everything. In any case, we can figure it out together

alexrudd2 · 2024-02-14T05:26:05Z

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

janiversen · 2024-02-14T07:40:14Z

@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

Changes look good, feel free to merge once CI is green.

I updated CI some days ago, and solved a pylint issue yesterday, so just rebasing on newest dev might please CI.

pyproject.toml

janiversen · 2024-02-14T07:56:04Z

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

dhoomakethu · 2024-02-14T08:24:49Z

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 · 2024-02-14T08:31:53Z

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 · 2024-02-14T08:49:35Z

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 · 2024-02-14T09:02:20Z

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 · 2024-02-14T09:35:57Z

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 · 2024-02-14T10:12:10Z

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 · 2024-02-14T12:01:15Z

but it works normally as you can see on dev.

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

janiversen · 2024-02-14T13:37:53Z

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

dhoomakethu · 2024-02-14T14:05:32Z

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 · 2024-02-14T14:31:47Z

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 · 2024-02-14T15:27:53Z

good catch, I did not think of that.

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

janiversen · 2024-02-14T15:28:13Z

If this one get green, then lets merge.

janiversen · 2024-02-14T15:44:50Z

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 · 2024-02-14T15:47:21Z

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

janiversen · 2024-02-14T15:54:06Z

OK, looking forward to see that PR.

dhoomakethu requested review from janiversen and alexrudd2 February 13, 2024 14:31

janiversen requested changes Feb 13, 2024

View reviewed changes

pyproject.toml Show resolved Hide resolved

alexrudd2 approved these changes Feb 14, 2024

View reviewed changes

janiversen approved these changes Feb 14, 2024

View reviewed changes

pyproject.toml Show resolved Hide resolved

pyproject.toml Outdated Show resolved Hide resolved

janiversen force-pushed the repl-as-plugin branch from 2d3e455 to b8e0a09 Compare February 14, 2024 12:02

dhoomakethu and others added 5 commits February 14, 2024 14:36

Move repl to a seperate repo

6b16ecf

Move repl test to out to repl repo, update readme

20af39f

Update documents.

7b40c95

Update documentation - 2

4b43527

Remove REPL.rst

ef838c8

test.

f683f88

janiversen force-pushed the repl-as-plugin branch from 5c9248e to f683f88 Compare February 14, 2024 13:40

Fix CI build issues

dd9e043

remove repl aiohttp dependency.

c1dd451

janiversen merged commit b4490ab into dev Feb 14, 2024

janiversen deleted the repl-as-plugin branch February 14, 2024 15:44

github-actions bot locked as resolved and limited conversation to collaborators Feb 25, 2024

Move repl to a seperate repo #2009

Move repl to a seperate repo #2009

Uh oh!

Conversation

dhoomakethu commented Feb 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Uh oh!

janiversen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

janiversen commented Feb 13, 2024

Uh oh!

janiversen commented Feb 13, 2024

Uh oh!

alexrudd2 commented Feb 13, 2024

Uh oh!

dhoomakethu commented Feb 13, 2024

Uh oh!

dhoomakethu commented Feb 13, 2024

Uh oh!

janiversen commented Feb 13, 2024

Uh oh!

janiversen commented Feb 13, 2024

Uh oh!

dhoomakethu commented Feb 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhoomakethu commented Feb 14, 2024

Uh oh!

alexrudd2 left a comment

Choose a reason for hiding this comment

Uh oh!

alexrudd2 commented Feb 14, 2024

Uh oh!

janiversen commented Feb 14, 2024

Uh oh!

janiversen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

janiversen commented Feb 14, 2024

Uh oh!

dhoomakethu commented Feb 14, 2024

Uh oh!

janiversen commented Feb 14, 2024

Uh oh!

dhoomakethu commented Feb 14, 2024

Uh oh!

janiversen commented Feb 14, 2024

Uh oh!

dhoomakethu commented Feb 14, 2024

Uh oh!

dhoomakethu commented Feb 14, 2024

Uh oh!

janiversen commented Feb 14, 2024

Uh oh!

janiversen commented Feb 14, 2024

Uh oh!

dhoomakethu commented Feb 14, 2024

Uh oh!

dhoomakethu commented Feb 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

janiversen commented Feb 14, 2024

Uh oh!

janiversen commented Feb 14, 2024

Uh oh!

janiversen commented Feb 14, 2024

Uh oh!

dhoomakethu commented Feb 14, 2024

Uh oh!

janiversen commented Feb 14, 2024

Uh oh!

Uh oh!

dhoomakethu commented Feb 13, 2024 •

edited

Loading

dhoomakethu commented Feb 14, 2024 •

edited

Loading

dhoomakethu commented Feb 14, 2024 •

edited

Loading