-
Notifications
You must be signed in to change notification settings - Fork 390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Refactor PlcDriverManager as context manager #124
Conversation
@JulianFeinauer is there any particular reason for the structure of |
@nuclearpinguin first off, hi Thomascz, nice to have you here : ) And in fact, when we started with python (no one had a clue about python), @chrisdutz and I made the structure as we know it from java / maven. So thats rather historic reason. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with all you changes. In fact the whole python setup is WIP and we are happy for "native" pythoners that support us there!
Hi @nuclearpinguin, I always thought that it would be great if someone who actually knew a language would at least help with the API in order for it to feel right. So I'm fine with any changes you propose. However it would be great if it would stay possible to build the project with maven as the code generation will have to be done with maven and as long as all is located in one repo, that we can do a CI build with all parts in one run. Chris |
@chrisdutz can I find somewhere detailed information on how the code generation works? @JulianFeinauer thanks for the info, I will refactor the PLC4py. I will also familiarize myself with maven and the whole project structure. |
I'd suggest starting here: https://plc4x.apache.org/developers/code-gen/index.html |
@@ -18,6 +18,7 @@ | |||
import subprocess | |||
import time | |||
import warnings | |||
from contextlib import contextmanager | |||
|
|||
from generated.org.apache.plc4x.interop.InteropServer import Client, PlcException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JulianFeinauer can you explain the import? I can't find the generated.org...
package nowhere in the project and I am wondering what I've missed... I was able to build the project but only some target
directories appeared.
I had this info during the build:
-- Building without tests
-- Found FLEX: /usr/bin/flex (found version "2.6.4")
-- Found BISON: /usr/bin/bison (found version "3.0.4")
-- ----------------------------------------------------------
-- Thrift version: 0.13.0 (0.13.0)
-- Thrift package version: 0.13.0
--
-- Build configuration summary
-- Build compiler: ON
-- Build libraries: ON
-- Build tests: OFF
-- Build type: RelWithDebInfo
--
-- Language libraries:
--
-- Build as3 library: OFF
-- - Adobe Flex compc was not found - did you set env var FLEX_HOME?
--
-- Build C++ library: OFF
--
-- Build C (GLib) library: OFF
-- - GLib missing
--
-- Build Java library: OFF
--
-- Build Python library: OFF
-- - Python libraries missing
--
-- Build Haskell library: OFF
-- - GHC missing
-- - Cabal missing
--
-- ----------------------------------------------------------
-- Configuring done
-- Generating done
But I'm not sure if this is somehow related (especially the Haskell part :D ).
The missing code is generated by thrift as part of the maven build. |
I would like to suggest to use
PlcDriverManager
as a context manager. In this way users don't have to care about try / excepts. I also extracted some server logic from constructor to separate method.I will add some tests but first I have to sort out setup :)