docs: Proposal for FederatedCatalog Distribution and TargetNodeDirectory #1555
Conversation
|
This pull request is stale because it has been open for 7 days with no activity. |
There was a problem hiding this comment.
Generally, before going into details I want to clarify a few things. A decision record should document a Decision (not options) and have the following structure:
- Decision: outline in a succinct and concise manner, in one or two sentences, what was decided.
- Rationale: outline the thought process how that decision was reached, maybe mention one or two alternatives and why they were not chosen. Motivate your decision.
- Approach: how the decision can be implemented. This should also outline possible limitations.
Specifically, the FC is not a "standalone service". What you probably mean is a separate runtime (docker image, runnable JAR file) and thus a separate deployment in the Tx-EDC Helm Chart, like the data plane. It is extremely important to use precise wording
paullatzelsperger
added
documentation
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
| The Federated Catalog aims at providing the aggregation of catalogs from multiple participants in a centralized point to reduce latency. | ||
|
|
||
| Choosing a solution that decouples from Control Plane (like the one used for the Data Plane) and able to be scalable will future-proof the Federated Catalog as a feature and embraces wider usage. | ||
| Like so, having a Federated Catalog service, scalable on its own based on user needs, and not on Control Plane, allows the best resource management in the long term. It also enables adding features to the Federated Catalog without the need to change the Control Plane logic (assuming no API usage change). |
There was a problem hiding this comment.
the first part is already described by the previous sentence, the second one is not 100% correct, because it is actually possible to add features to the control plane without change its "logic" (depending on what do you mean with "logic", but generally speaking that's what the EDC modular architecture is about)
There was a problem hiding this comment.
This was bad phrasing on my side, I was aiming at mentioned the goal of having independent updates. However, in the previous paragraph was already mentioned the decoupling this approach promotes so it (like the scalability) can be seen as repetitive information.
Removed in here.
There was a problem hiding this comment.
in any case also the "indipendent updates" is not something we want to achieve with this decision, because we will release the FC together with CP and DP, otherwise we would put it in a separate chart.
There was a problem hiding this comment.
I see. I wanted to highlight, as an example, that if a change in the Federated Catalog Cache or similar should be transparent for other instances (like CP or DP). And it would be, I believe, but updating to a new version when all are in tractus-edc charts then "independent update" is not accurate.
Thanks!
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
|
Considering this suggestion, the Decision Record proposals for the Distribution and the TargetNodeDirectory were merged in one since they related within same scope. |
bmg13
changed the title
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
… into dr_proposal_federated_catalog_distribution
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Outdated
Show resolved
Hide resolved
docs/development/decision-records/2024-09-16-proposal-federated-catalog-distribution/README.md
Show resolved
Hide resolved
… into dr_proposal_federated_catalog_distribution
|
This pull request was closed because it has been inactive for 7 days since being marked as stale. |
|
@jimmarino @ndr-brt @paullatzelsperger can someone reopen this pr as well, please? |
… into dr_proposal_federated_catalog_distribution
|
Updated the documentation to include DID input, having the BDRS resolve of those DID and call the Discovery Service to obtain the Connector URL's. This is aligned to what was discussed in a weekly meeting. |
|
This pull request is stale because it has been open for 7 days with no activity. |
|
sorry for reping, but @jimmarino @ndr-brt @paullatzelsperger @wolf4ood can someone look at this pr, please? |
Can you resolve the outstanding comments and we can re-review? |
… into dr_proposal_federated_catalog_distribution
Sorry about it, all seems addressed now, thank you! |
The comments are not resolved in GitHub, which makes it difficult for reviewers to track what was applied. Please do that before requesting a re-review. |
I usually leave the creator of the comment to resolve it. But I understand, resolved it in the meantime all that are resolved, left a couple open since is ongoing discussion |
| For TargetNodeDirectory it will be set by a new extension responsible for exposing an API, where a member can input the DID's of the participants from which the catalogs are wanted, and then it will retrieve and store the respective Connector URL's. This new extension would get the data from the Discovery Service, and will be named `DiscoveryServiceRetrieverExtension`. This solution allows the member to choose precisely the Target Catalog Nodes that interests them, resulting in reduced network calls and latency. | ||
| Additionally, if a Connector URL is registered (or unregistered) in the Discovery Service, the retriever will reflect it since it requests based on BPN and the registered URL's will be returned. | ||
|
|
||
| This solution improves on the default one of having the data in a static file since a dynamic approach would avoid downtime when a change is required. | ||
|
|
||
| Other solution for the TargetNodeDirectory was also considered | ||
| - File in a S3 bucket (or different cloud provider's solution) | ||
| - This solution was discarded due to one file for all instead of each partner having the data that respectively needs does not match the requirement and this solution would lock the usage of a proprietary tool (cloud provider) being harder to sustain in the long run. |
There was a problem hiding this comment.
this is more of a solution proposal and thus should be moved to the "Approach" section if not already there.
There was a problem hiding this comment.
I thought that, despite being a suggestion of a solution to be followed and one not be followed this would make sense as a Rationale. But followed that suggestion and transplanted it to the "Approach" section.
Changed in here.
|
|
||
| Since the Federated Catalog will be a standalone runtime, the Tractus-X EDC Connector Helm charts will be updated to include the Federated Catalog as a separated deployment. The update will include the creation of a specific `deployment-federatedcatalog.yaml`, similar [to this one](https://github.com/eclipse-tractusx/tractusx-edc/blob/a263bf71a110245657131509d4b37d058a1d220d/charts/tractusx-connector-azure-vault/templates/deployment-dataplane.yaml#L47) (for `ingress` and `hpa` as well), for different scenarios (InMemory, PostreSQL, etc.). This results in added configuration complexity. | ||
|
|
||
| To enable the Federated Catalog flow, please [see this table](https://github.com/eclipse-tractusx/tractusx-edc/blob/75bdacbad43e2cad352204ea28a359c6aac7adea/docs/development/management-domains/README.md#enable-and-configure-the-crawler-subsystem). |
There was a problem hiding this comment.
spurious. how we configure an FCC is not relevant for this Decision Record.
There was a problem hiding this comment.
I understand, kept the first paragraph since believe it to add relevant context here, but the enabling config was removed.
Removed in here.
| To enable the Federated Catalog flow, please [see this table](https://github.com/eclipse-tractusx/tractusx-edc/blob/75bdacbad43e2cad352204ea28a359c6aac7adea/docs/development/management-domains/README.md#enable-and-configure-the-crawler-subsystem). | ||
|
|
||
| For its TargetNodeDirectory, the extension is able to obtain the Connectors' URL's through the Discovery Service and store them. Two API's will be provided in this new extension, at least during alpha stage, one to allow the user to input a list of DID's and other for BPN's. The `DiscoveryServiceRetrieverExtension` is responsible to retrieve the data and store it (in memory or in a database). The URL's can later be retrieved and crawled by the Federated Catalog. | ||
| If no DID is saved (to be crawled) then the extension will not request data from the Discovery Service, since the number of catalogs in the space can be significant. |
There was a problem hiding this comment.
We don't need to specific what doesn't happen :)
I'd rephrase this to something like: "By default, the list of TargetNodes is empty..."
There was a problem hiding this comment.
this was left to abide to an ask on what default expected behaviour if no DID is stored. But I see your point. Kept it, but following your "by default" suggestion.
Changed in here.
|
|
||
| Some limitations of this TargetNodeDirectory solution are: | ||
| - Each partner must have the DID's beforehand. If a new Partner is registered and an existing partner would want their catalog, the DID (or BPN) of the new partner must be obtained first and added through the new extension API; | ||
| - Deal with the overhead an additional persistence store; |
There was a problem hiding this comment.
what does this mean? how is this a limitation? If anything, it means that we'll need an in-memory and an SQL variant of this store.
There was a problem hiding this comment.
You're right, this will be a limitation if a SQL store is used, but InMem or SQL options are provided as other cases and this is not a valid limitation here.
Removed in here.
| { | ||
| "bpn": "BPNL000000000002", | ||
| "connectorEndpoint": [ | ||
| "https://connector2/api/v1/dsp", |
There was a problem hiding this comment.
i would caution against using the BPN as primary key. Moving away from BPNs and toward DIDs fully is currently being discussed and this would cause problems with the data model later on. In addition, conflating business keys (such as BPNs) and database keys is somewhat of an antipattern.
There always is the possibility to wrap the TargetNode in another class and use that class as database entity.
|
Please lets be very specific with the wording:
With that said, IMO the new runtime should be called "Federated Catalog Cache", not "Federated Catalog". There may also be "Federated Catalog Servers" in the future (-> Management Domains). |
… into dr_proposal_federated_catalog_distribution
|
Thank you for that, it makes sense. |
WHAT
Decision Record for new proposal for FederatedCatalog distribution (with Tractus-X) and the TargetNodeDirectory (TND).
The TND initial proposal was defined in this PR and continue in the current one.
Related with #1585