International-Data-Spaces-Association / IDS-testbed

Apache License 2.0
24 stars 32 forks source link

IDS Test bed fix #143

Closed parwinderau closed 2 months ago

parwinderau commented 3 months ago

There are three fixes identified:

  1. At the docker-compose.yml level where version is missing and broker-reverse-proxy should be used instead of local host, so that microservices can be discovered to each internally.
  2. At the .env level use of quotes creates problem. So better to use without quotes or with absolute path.
  3. At the postman level where parsing of 'Consumer Agreement Id' is fixed with generic code. Now it can run agnostically.
jfernandezsqs commented 3 months ago

I have reviewed the changes made in this pull request and the modifications made to the docker-compose.yml and .env files are correct and I have verified that they do not affect the functionality or interoperability of the environment.

As for the postman collection TestbedPreconfiguration.postman_collection.json I have detected an error in lines 645 and 661. This failure is due to the {{BROKER_REV_PROXY}} variable which is not defined and enabled in the active environment (UNRESOLVED variable). Is the error due to this missing variable at the collection? @parwinderau Could you please fix the error in the collection?

Thanks in advance ;)

parwinderau commented 3 months ago

I have fixed the undefined BROKER_REV_PROXY variable.

parwinderau commented 3 months ago

I have reviewed the changes made in this pull request and the modifications made to the docker-compose.yml and .env files are correct and I have verified that they do not affect the functionality or interoperability of the environment.

As for the postman collection TestbedPreconfiguration.postman_collection.json I have detected an error in lines 645 and 661. This failure is due to the {{BROKER_REV_PROXY}} variable which is not defined and enabled in the active environment (UNRESOLVED variable). Is the error due to this missing variable at the collection? @parwinderau Could you please fix the error in the collection?

Thanks in advance ;)

Done now. Please check it. BR Parwinder

jfernandezsqs commented 3 months ago

I have checked the changes and from my side they are fine. I am going to make some minor changes to the documentation to reflect the changes made.

parwinderau commented 3 months ago

Changes look good to me. Therefore, I approve this pull request.

Thanks. For merging I think there is a need for a second reviewer also. Can you ask someone or assign someone for a second review as well.