Skip to content

Conversation

@EricLo-417
Copy link

Hello Devs,

I have been tasked with implementing connected systems and came across your repo. However, it seems that your readme and some of the code is a little out of date and did not work out-of-the-box.

Figured I would make a merge request of the small changes I did to get the project from zero to hero.

Disclaimer: I don't really do Python, so I may have misunderstood some of the python code changes. Also, the readme info may be unnecessary to python devs, but it helped start the project from scratch so figured it may be worth including.

...

The main issues I ran into

  • How to set up the env. Project mentions requriments.txt but I think it may worth showing how to set up a virtual env for portability. (Again, maybe this isn't standard practice for python devs but as a node/java dev this made sense)
  • How to actually build the project. The readme references files that no longer exist.
  • How to set up the external deps. Docker compose is helpful but not so much if you wanted to run project locally. Included a separate docker compose with only external deps and ports
  • Once everything got stood up, the simulator script errored.
    • Basic auth was wrong. The sample .env does not provide the basic auth variables so they default to secrets.token_hex(32) not test. So I added them to the .env.sample. I also turned off basic by default in the .env.sample anyways as that just made quick dev testing easier. But I can easily change it back to "true" if basic should be turned on by default
    • Once that was fixed, system creation failed due to 'rel'. typeOf["rel"] fails if the typeOf object does not have a rel; which the default system does have. I could have added it to the system post json request but I think the intention was make it optional, so I turned into a safe get field
    • Lastly similar issue with the datastream item["deployment@link"] fails if deployment@link is not set. This time I could get away with get to safely get the field value. Again, I assumed the intention was this should be optional.

@SpeckiJ SpeckiJ self-requested a review September 30, 2025 06:52
@SpeckiJ
Copy link
Member

SpeckiJ commented Sep 30, 2025

Hello,

thanks for your contribution, unfortunately I will have to check with some colleagues first before I review+merge this (due to licensing regulations).

However, it seems that your readme and some of the code is a little out of date and did not work out-of-the-box.

I admit that I neglected updating the README and the repository still lacks a real "onboarding/howto" guide - sorry for that. We are aware , this is just a priorization issue and unfortunately we are currently very low on project resources for developing+maintaining the implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants