📣 Future Plan for Brick Server

[Jason Koh]

Dear all,

There have been many inquiries around Brick Server but due to my limited cycles, the support, both communication and development, has been sparse. We decided to take two actions for helping users in the right direction.

- We change the name from Brick Server to Brick Example Server.

Many of the Brick users have been looking for how to use Brick with their own data and systems. We proposed Brick Server as an example including an API spec, a timeseries database, and a triple store. We designed it deliberately, but it doesn't have to be the single way of using Brick, but the name "Brick Server" often misleads users to consider it as the only way to use Brick, which is not true. Thus, we decided to change it to Brick Example Server, and the repository URL has been changed to https://github.com/BrickSchema/brick-example-server as well. The old URL will be redirected to there.

- Xiaohan Fu (https://github.com/reapor-yurnero) volunteered to be an active maintainer of Brick Example Server.

I personally did a poor job at responding to issues and maintaining the project. Xiaohan gladly took it over for the Brick community, along with his own research. As a first step, he has refactored and documented the code better. Most of the things should be working as described in the README. If you were waiting for some fixes, please try it out and share your feedback.

We appreciate all the feedback and the interest in Brick Server, and hope this announcement clarifies how to use Brick and resolve your questions. Let us know if you have any other questions and feedback.

Thank you.

[Scott Palmer]

So I cloned the repo yesterday and tried some things out.  Here are some notes and comments.

I was able to:

1.  create the brick-server:minimal image just fine (docs are good)
   • a windows example should have been provided
     • set DOCKER_BUILDKIT=1
2. bring up the image set with docker-compose (docs are good)
3. reach the Swagger-UI page without issue
4. create a JWT just fine (docs are good)

I was not able to:

1. run all the tests

So I did a bit of a deep dive on that since I had the time. My comments and opinions are below. I am posting here to see what the consensus / feeling is about them before I prepare a MR on GitHub for example.

Comments / suggestions / criticism and all that is welcome and expected.  And yes, I recognize that some of the changes below are very small and perhaps more just opinion than better.

In no particular order:

1. fix dev dependencies in pyproject.toml
   1. [tool.poetry.dev-dependencies] should contain (that's what it is for) (they are not options) : 
      • asgi-lifespan
      • pytest
      • pytest-depends
      • pytest-asyncio
      • pytest-env
      • pytest-ordering
2. tests should be separated from production code
   
   1. move /brick_server/minimal/tests to /tests
   2. the /tests folder should not contain __init__.py
      1. test files should generally not import functional code from other files in the test folder
         1. if you do, you are basically depending on untested code
         2. if you need a lot of code to do a test it should probably be another project that you pip install and test all on its own
      2. importing constants is okay
      3. pytest dependency injection should be used where appropriate
         
      4. use .env and .env.local for magic / special values in tests
   3. file and folder paths should always be referenced from a known ROOT (typically PROJECT_ROOT)
      1. place this in /tests/common.py
         from pathlib import Path
         
         PROJECT_FOLDER = Path(__file__).parent.parent.absolute()
         EXAMPLES_DATA_FOLDER = PROJECT_FOLDER / "examples" / "data"
         assert EXAMPLES_DATA_FOLDER.is_dir()
         
   4. use the EXAMPLES_DATA_FOLDER constant with open()
      • current code has hidden current working path expectation; example:
      • with open("examples/data/bldg.ttl", "rb") as fp:
      • change to
        • with open(EXAMPLES_DATA_FOLDER / "bldg.ttl", "rb") as fp:
3. class names should not be redeclared as done in asyncpg_timeseries.py (you are asking for trouble when the IDE helps you refactor and it choose the wrong class)
   • just change:
   1. from brick_data.timeseries import AsyncpgTimeseries
   • to
   1. from brick_data.timeseries import AsyncpgTimeseries as BaseAsyncpgTimeseries
4. the code in minimal/interfaces/__init__.py seems overly complicated for no gain.
   1. additionally the `# nopycln: file` statement seems to have removed some lines from that file when I moved the tests on my local machine. (I was missing imports that were originally there.)
      1. Perhaps if what is to be part of interfaces should be dynamic, a plugin architecture should be used.
      2. pycln seems dangerous as currently used; luckily git status would pick up any undesired behavior
5. as per above use .env and .env.local instead of hard-coded names like this one:
   • async with AsyncClient(app=app, base_url="http://testserver") as c:
6. I see tools in the pre-commit hook that are not present in [tool.poetry.dev-dependencies] 
   • shouldn't they be included?
   • I too am a fan of isort and black
     • the config for both of these tools seems missing from the repo
       • perhaps that too should be added to the repo

I am super excited and impressed with what has already been provided. Thank you and Amazing work!  and thank you again

I'd like to see the code base a little more "industry standard" (see comments above) so it is easier for others, such as myself, to dive in and get up and running.

I am willing to contribute.  

DISCLAIMER: new to Brick schema and some of the technologies used in this project, not to software development and web stacks such as this one

[Xiaohan Fu]

Not sure why my previous message disappeared. I'm very glad to see all of your comments and suggestions! I will take a look soon and get back to you. We are not real software engineers though we are trying to make things as standard as possible in this recent update so we would be more than happy to learn from the community! Thank you!

[Oisín Grehan]

I also ran into this:

Docker-compose configuration doesn't work out the box · Issue #25 · BrickSchema/brick-example-server (github.com)

I got my admin jwt issued from the brick-server container to play with the openapi/swagger, but the entity api calls would just hang forever. The fix above (bumping max connections) help me get one api working on the swagger page (list all entities), but further efforts just hung the page with other attempts to call APIs (i.e. get a single entity with its entity_id garnered from the list).  

This did not aid my confidence in investigating further, so I find myself here.

[Yihao Liu]

Sorry for the late reply. As one of the active developer of this project, thanks for all of your suggestions! I'll reply to them one by one.

1. changed

2. most are fixed, except "__init__.py" since common.py still need it. I'll try to find out that how much we can be independent on untested code, and add more tests in the future.

3. changed

4. I think the code in minimal/interfaces/__init__.py is useful and can make the imports eaiser in other code.

"import xxx as xxx" is used because implicit import is not recommanded by PEP. I'm not sure why pycln is buggy on your side, maybe I'll remove it later.

It is added mainly because when I rewrite this project, many not referenced imports exist and I used pycln to remove them efficiently.

5. "http://testserver" is just a placeholder for inner usage, I copied this from the documentation of fastapi: https://fastapi.tiangolo.com/advanced/async-tests/

6. pre-commit hooks are managed by pre-commit itself, not by the project. After you run "pre-commit install", the tools will be installed in "~/.cache/pre-commit" by default. If the package includes these tools, they'll still be installed again in the cache directory by pre-commit, so there is no need to include them in the project.toml. I think the default config is used for black and isort now, do you have some recommandation of some uesful config items for them?