feat: add logging-house-client

[schoenenberg]

What issues does this PR close?

Add placeholder logging-house-client

Checklist

Beta Give feedback

1. The PR title is short and expressive.
2. I have updated the CHANGELOG.md. See changelog_update.md for more information.
3. I have updated the Deployment Migration Notes Section in the CHANGELOG.md for any configuration / external API changes.
4. I have performed a self-review

[schoenenberg]

Hi,
please set the USERNAME & TOKEN env variable also for the grade build stage as requested in here: https://github.com/sovity/edc-extensions/pull/732/files#diff-c0dfa6bc7a8685217f70a860145fbdf416d449eaff052fa28352c5cec1a98c06R84

[tmberthold]

A few questions and comments which need to be adressed:

• Repo with source-code: https://github.com/truzzt/mds-ap3
• There's no documentation for the extension, not here and not in the source-code-repo.
• Which logging house are you aiming to be compatible with in times of DSP messages, is it already deployed somewhere?
• How was functionality tested, looking at the code the extension does nothing else than just print if it was enabled/disabled??
• The build.gradle of the linked repo contains dependency-repos from IAIS, which, as we know, have been deactivated a long time ago, needs to be removed (why is it even there?).
• The IDS-mulitpart-protocol and its logMessage are in the requirements file in the repo (we have to look somewhere to see what you're planning). These messages no longer exist in the days of the DSP-protocol. This contents need to be updated.
• We have to find a solution on how you can address breaking changes in an event of an edc-core update in this repo. How did you plan the process? Wouldn't you rather contribute the code itself here or do you update the code and publish a new LH-maven-package when there are breaking changes?

As long as the code/extension itself has no functionality other than the fact that you can switch it on and off and this is logged, we will not further consider merging the PR into main for the time being.

//cc: @jkbquabeck @richardtreier @SebastianOpriel

[dhommen]

Hi @tmberthold ,
due to sovity's availability it was requested by @jkbquabeck that sovity needs the extension until January 19 to be able to integrate it in time.
Therefore we agreed with sovity and MDS to deliver a placeholder extension, so that sovity work on the integration while we are developing the actual extension and the required docs.
It is your decision when you are integrating the extension, but remember it was on sovity's request to start as early as possible with the integration and it is your responsibilty to integrate it within your and the MDS deadline.

Second, formally the implementation details of the extension are since the takeover of the client extension not in your scope anymore. We have today a meeting with the MDS and will discuss your suggestions.

Please keep in mind that the logging-house-client extension aims to work and be updated according to the MDS roadmap.
If you no longer need to be able to integrate the extension beforehand feel free to close this PR.

[jkbquabeck]

Hi @dhommen ,
you are correctly stating that I agreed to you raising the PR for the placeholder extension. The 19th of January was still the last day to deliver the full logging house extension and making it publicly available. The reasoning behind this was that we need to be able to assess the extension that shall be included into our repository. Happy to discuss this with you and MDS in a brief meeting if there is a need for that.

[dhommen]

Hi @jkbquabeck,
this is not what was agreed on in the Bi-weekly with the MDS and you. We will escalte this to the MDS.

[tmberthold]

Hi, please set the USERNAME & TOKEN env variable also for the grade build stage as requested in here: https://github.com/sovity/edc-extensions/pull/732/files#diff-c0dfa6bc7a8685217f70a860145fbdf416d449eaff052fa28352c5cec1a98c06R84

A few technical things here, if I remeber corretly how it normally needs to be done:

• Since this PR is from your repo, you need to edit the ci.yml and add the env-settings there in the gradle-build step (name: "Gradle: Build")
  - I already added the secrets "GH_MAVEN_REG_USER" and "GH_MAVEN_REG_TOKEN" to this repo here (token is valid for the next 7 days). If you want me to rename the secrets here in this repo, just give me a ping.
  - Note: We can't use the default Github-Actor and -Token from this PR/Repo because your GH-Maven-Registry is not associated with this repo/orga, so we need to use individual secrets to access your GH-Maven-Registry from this repo/orga. This default settings works for your repo for the gradle build step but will not work for external repos which need to access your GH-Maven-Registry like this one.
  - But even if it's done it will most likely not work since GitHub's restriction "They [secrets] are not passed to workflows that are triggered by a pull request from a fork.", which is here the case since your repo is a fork of this one.

If there is another/better approach in this construction of this PR, please let me know.

Update: see comments below

[dhommen]

Hi @tmberthold,

Based on my understanding of GitHub workflows, using the GITHUB_TOKEN appears to be sufficient. The documentation indicates that the GITHUB_TOKEN has limitations only in PRs from private forks. I successfully conducted a build job by forking our repository. You can check the PR here. Although it fails for the Docker Dev image, this seems to be a repo configuration issue. The changes have been pushed.

[tmberthold]

Hi @tmberthold,

Based on my understanding of GitHub workflows, using the GITHUB_TOKEN appears to be sufficient. The documentation indicates that the GITHUB_TOKEN has limitations only in PRs from private forks. I successfully conducted a build job by forking our repository. You can check the PR here. Although it fails for the Docker Dev image, this seems to be a repo configuration issue. The changes have been pushed.

Apparently your understanding is correct - thanks for adding the env properties to the ci!

[dhommen]

Hi @tmberthold,
Based on my understanding of GitHub workflows, using the GITHUB_TOKEN appears to be sufficient. The documentation indicates that the GITHUB_TOKEN has limitations only in PRs from private forks. I successfully conducted a build job by forking our repository. You can check the PR here. Although it fails for the Docker Dev image, this seems to be a repo configuration issue. The changes have been pushed.

Apparently your understanding is correct - thanks for adding the env properties to the ci!

I need to clarify my previous statement: the GITHUB_TOKEN is set to read-only permissions. Assuming that the docker images aren't required, I've disabled the Docker Image: and the add_pullrequest_to_project jobs for fork PRs. This change should remove the need for write permissions.

[tmberthold]

I created a GHCR-Image via another PR including this extension which can be used for testing (based on v0.1.0):
https://github.com/sovity/edc-extensions/pkgs/container/edc-ce-mds/170907241?tag=pr-750

The default case without setting the env-props or if it was explicitly switched off, the extension logs that it is switched off:
edc-1 | 2024-01-25 09:12:16 Logginghouse client extension is disabled.

If CLEARINGHOUSE_CLIENT_EXTENSION_ENABLED: "true" and EDC_CLEARINGHOUSE_LOG_URL (any value) are set, the extension logs, that it has been enabled:
edc-1 | 2024-01-25 09:14:04 Logginghouse client extension is enabled.

So technically this "placeholder extension" (v0.1.0) works as expected, given the current functionality.

[SebastianOpriel]

How could we enable Dependabot to automatically create PRs if there were any dependency updates. Does this make sense?

[dhommen]

How could we enable Dependabot to automatically create PRs if there were any dependency updates. Does this make sense?

The current version of the logging-house-client-extension is in the 0.x.x series, suggesting that not every version update necessarily warrants immediate action. Therefore, I recommend postponing discussion on this topic until we reach version 1.0.0.

[dhommen]

Hello @tmberthold I have reverted the rebase from main and only add the version update for the extension. How do you want to proceed with the conflicts of the workflow files that we changed to enable GH actions from fork PRs?

[tmberthold]

Hi @dhommen,

• I think you still have to update the fork and this PR-branch against our main somehow, I guess one of the conflicts of this PR is for example coming from changes done to add_pullrequest_to_project.yml in this PR, which was totally removed on this repos main in the meantime, so there are attempted changes to a file in this PR that no longer exists at all on the main.
• The changelog in this PR still includes "Add logging-house-client extension 0.1.0" while v0.2.9 is integrated, same for the env-names under "Deployment Migration Notes", I guess the env-names could be updated too if they changed with v0.2.9, I also suspect that this file also needs to be updated against our main, as it seems to be out of date on this PR (e.g. releases 7.2.1 and 7.2.2 missing).
• just out of interest: I saw there's already a v0.2.10 out, should we stay with v0.2.9?
• I will discuss the adjustments made to the other workflows internally. They basically switch off the action-steps when a PR comes from a fork, which makes sense because a PR from a fork cannot push images into our GHCR. I'll report back if changes are necessary.

Thank you for the update!

[dhommen]

Hi @tmberthold
we have updated the logging-house-client to version 0.2.10 and made the necessary changes in the CHANGELOG.
The Connector 7.3.0 including the extension was successfully tested in the MDS dev system.

[tmberthold]

Hi @dhommen,

I discussed the following internally:

I will discuss the adjustments made to the other workflows internally. They basically switch off the action-steps when a PR comes from a fork, which makes sense because a PR from a fork cannot push images into our GHCR. I'll report back if changes are necessary.

Could you do us a favor and:

• change the target-branch of this PR to another branch in this repo which is already existing and we just created for this: feat/lh-extension (if you can't due to permissions just let me know)
• remove the 4 disabling if's in ci.yml? if: ${{ github.event.pull_request.head.repo.full_name == github.repository }}

If that would be possible, that would be great!
//cc: @jkbquabeck

[dhommen]

Hi @tmberthold could you please elaborate a little bit more on why the change to another branch is needed and the ci.yml adjustments should be reverted when it first was requested for the action to pass?

[tmberthold]

Hi,
We would like to have a test-image of the changes in our GHCR before merging it into our main, which is of course not possible with the deactivated steps on this branch/PR (due to the fork). In order to speed things up a little, we will continue this integration ourselves from here, so that should be done for you for now, if there are no technical problems with/questions for the extension itself.
Thank you very much!