chore: Remove knora-ontologies symlink (DEV-3236)

[siers]

Pull Request Checklist

Task Description/Number

Issue Number: DEV-

PR Type

• build/chore: maintenance tasks (no production code change)
• docs: documentation changes (no production code change)
• feat: represents new features
• fix: represents bug fixes
• perf: performance improvements
• refactor: represents production code refactoring
• test: adding or refactoring tests (no production code change)

Basic requirements for bug fixes and features

• Tests for the changes have been added
• Docs have been added / updated

Does this PR introduce a breaking change?

• Yes

Does this PR change client-test-data?

• Yes

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (eac5751) 11.74% compared to head (c7b433d) 88.76%.
Report is 64 commits behind head on main.

Files	Patch %	Lines
...src/main/scala/dsp/valueobjects/LanguageCode.scala	66.66%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #3035       +/-   ##
===========================================
+ Coverage   11.74%   88.76%   +77.02%     
===========================================
  Files         246      262       +16     
  Lines       22907    22806      -101     
===========================================
+ Hits         2690    20244    +17554     
+ Misses      20217     2562    -17655     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[siers]

@seakayone The resetting from docker now works, but I don't think it ever worked while running directly from SBT, at least, I can't see how it could have, because it was relying specifically on the docker mounted paths.

EDIT: this is not quite the case, because TriplestoreServiceLive loads them locally as well, so I'll have to think about the fix a bit.

[siers]

@seakayone Some advice would be helpful, here's the problem:

The knora-ontologies and test_data are loaded in local development in TriplestoreServiceLive.insertDataIntoTriplestore with cwd = .../dsp-api/integration, so from there the old paths were ../knora-ontologies (symlink) and ../test_data.

From the container, cwd = /opt/docker which is the unpacked jar. Both resource directories are mounted from the project directory with docker compose, making both ../knora-ontologies and ../test_data work, which is a lucky accident.

The DefaultRdfData contains a list of paths and introducing a path distinction for the deployment type would be weird.

Because of this having the symlink is a wonderful fix, however, if we do indeed want to get rid of it, I would suggest loading both of these resource directories as resources, have them baked in the jar for docker and accessible via regular resource loading mechanism in local dev. It would add 45M of content, however, but at least the access would be uniform.

What do you think about the proposition?

The non-uniform option would be to create a configuration after all, which would be 2x2 choices – local/container × knora-ontologies/test_data and then find the data according to the configuration table.

[seakayone]

@seakayone Some advice would be helpful, here's the problem:

The knora-ontologies and test_data are loaded in local development in TriplestoreServiceLive.insertDataIntoTriplestore with cwd = .../dsp-api/integration, so from there the old paths were ../knora-ontologies (symlink) and ../test_data.

From the container, cwd = /opt/docker which is the unpacked jar. Both resource directories are mounted from the project directory with docker compose, making both ../knora-ontologies and ../test_data work, which is a lucky accident.

The DefaultRdfData contains a list of paths and introducing a path distinction for the deployment type would be weird.

Because of this having the symlink is a wonderful fix, however, if we do indeed want to get rid of it, I would suggest loading both of these resource directories as resources, have them baked in the jar for docker and accessible via regular resource loading mechanism in local dev. It would add 45M of content, however, but at least the access would be uniform.

What do you think about the proposition?

The non-uniform option would be to create a configuration after all, which would be 2x2 choices – local/container × knora-ontologies/test_data and then find the data according to the configuration table.

As discussed, I would not like to have the test_data packaged in the container. Probably you can use the path to determine how to load. If it starts with knora-ontologies load it from the classpath, otherwise it remains a relative path as it is not.

[siers]

I was just once again reminded that there's the third scenario where these files get loaded, so to sum them up:

• cwd = dsp-api/integration, if you run sbt integration/test,
• cwd = /opt/docker (= unpacked jar), if you run it from the container, which has the data because of docker compose volumes,
• cwd = dsp-api, if you run sbt webapi/run with the env variable to allow reseting the triplestore content.

The third one never worked, so I haven't fixed it.

[siers]

Another interesting tidbit: the reset triple store endpoint is guarded by the allow-reload-over-http, which is defaulted to false, but gets set to true by the default compose configuration. If the default compose stack is used as the base for the compose config the production, then a mistake or a mistmatch of names in the production config wouldn't be type checked and would lead to more permissions, whereas it should be the case that the mismatch should lead to stricter permissions.

webapi/src/main/resources/application.conf:260
    allow-reload-over-http = false
    allow-reload-over-http = ${?KNORA_WEBAPI_ALLOW_RELOAD_OVER_HTTP}

docker-compose.yml:112
      - KNORA_WEBAPI_ALLOW_RELOAD_OVER_HTTP=true

[BalduinLandolt]

If that is re-used here, I wonder if it should still live in RepositoryUpdatePlan? It seems more general now.

[siers]

I am not entirely sure what would be best. It could be defined in DefaultRdfData indeed, if you wish me to move it.

[mpro7]

Don't forget to assign yourself to PR and to go through the checkboxes. If PR doesn't correspond to any issue/ticket, add short description under Task Description/Number section of the PR template, which tells what is the motivation behind it. Thanks!

[linear]

DEV-3236 dsp-api: Remove symbolic link for knora-ontologies

[siers]

@mpro7 I added the ticket number to the PR title, I'd truly forgotten it. However, I was wondering what the benefits of adding myself as an assignee are?

[mpro7]

@mpro7 I added the ticket number to the PR title, I'd truly forgotten it. However, I was wondering what the benefits of adding myself as an assignee are?

As I mentioned already in one of your former PRs, it's easier to serach through PRs if one is looking for something - it's possible to filter by assignee field too.

[siers]

@mpro7 I'm not sure there's much use since I'm already the author, but ok.

[mpro7]

@mpro7 I'm not sure there's much use since I'm already the author, but ok.

You're right, author works too, but it became our habit so why to change it? I find useful to find recent PRs in history of closed ones.

[siers]

@mpro7 Fair enough.