Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Update management-domains Federated Catalog docs. #1530

Conversation

bmg13
Copy link
Contributor

@bmg13 bmg13 commented Sep 3, 2024

WHAT

Suggestion update for Federated Catalog usage documentation.

WHY

To keep Federated Catalog documentation up-to-date.

FURTHER NOTES

Management Deployments Documentation was based in this one https://github.com/eclipse-edc/Connector/blob/main/docs/developer/management-domains/management-domains.md#type-2c-catalog-servercontrol-plane-with-data-plane-runtime

Related with
eclipse-tractusx/sig-release#736
#1585

@bmg13 bmg13 marked this pull request as ready for review September 4, 2024 11:22

![type 2b](https://github.com/eclipse-edc/Connector/blob/main/docs/developer/management-domains/distributed.type2.b.svg)
### Deployment Topologies
Copy link
Contributor

@paullatzelsperger paullatzelsperger Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is much value in copy-pasting documentation content that is already present in the upstream repo. In particular image material might be copyrighted there, so I'm not sure we even could legally copy it without attribution.

Instead, I would document how to set up a catalog server, i.e. which assets must be added there, how the resulting federated catalog looks like (flattened vs hierarchical), etc.

Copy link
Contributor

This pull request is stale because it has been open for 7 days with no activity.

@github-actions github-actions bot added stale and removed stale labels Sep 19, 2024
@ndr-brt ndr-brt self-requested a review September 27, 2024 06:47
Copy link
Contributor

github-actions bot commented Oct 5, 2024

This pull request is stale because it has been open for 7 days with no activity.

@github-actions github-actions bot added the stale label Oct 5, 2024
@github-actions github-actions bot removed the stale label Oct 8, 2024
Copy link
Contributor

This pull request is stale because it has been open for 7 days with no activity.

@github-actions github-actions bot added the stale label Oct 15, 2024
Copy link
Contributor

This pull request was closed because it has been inactive for 7 days since being marked as stale.

@github-actions github-actions bot closed this Oct 22, 2024
@bmg13
Copy link
Contributor Author

bmg13 commented Oct 23, 2024

@jimmarino @paullatzelsperger @ndr-brt can this PR be reopened, please?

@wolf4ood wolf4ood reopened this Oct 23, 2024
@wolf4ood wolf4ood removed the stale label Oct 23, 2024
Copy link

Copy link
Contributor

This pull request is stale because it has been open for 7 days with no activity.

@github-actions github-actions bot added the stale label Oct 31, 2024
@bmg13 bmg13 changed the title Feat: Update management-domains Federated Catalog documentation. Feat: Update management-domains Federated Catalog docs. Nov 7, 2024
@github-actions github-actions bot removed the stale label Nov 8, 2024
Copy link
Contributor

This pull request is stale because it has been open for 7 days with no activity.

Copy link
Contributor

This pull request is stale because it has been open for 7 days with no activity.

@github-actions github-actions bot added the stale label Nov 26, 2024
Copy link
Contributor

github-actions bot commented Dec 3, 2024

This pull request was closed because it has been inactive for 7 days since being marked as stale.

@github-actions github-actions bot closed this Dec 3, 2024
@bmg13
Copy link
Contributor Author

bmg13 commented Dec 9, 2024

@jimmarino @paullatzelsperger @ndr-brt can this PR be reopened, please?

Copy link
Contributor

@paullatzelsperger paullatzelsperger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, the text is quite misleading and IMO does not explain key concepts sufficiently.

I would have expected to see at least the following aspects mentioned:

  • the purpose of a catalog server, and what data structures it exposes (i.e. root catalog)
  • how to set up a catalog server, i.e. that each sub-catalog is referenced via a CatalogAsset and how such a CatalogAsset looks like. This is crucial and should be accompanied by an example. For reference, you can look at this example.
  • the fact that querying of a catalog server is no different from querying a "normal" catalog endpoint, and what the response looks like
  • the fact that contract negotiations etc. do still have to take place with the data-owning connector, not the catalog server
  • the fact that the catalog server doesn't actually create a consolidated catalog, i.e. it does not pull back any data, it merely references other connectors' catalog endpoints
  • the fact that the crawler lives on the consumer side
  • the purpose of a crawler and the fact, that it is the crawler that recursively traverses down a nested catalog
  • client-side caching: the crawler pulls back catalogs, stores them in the client-side cache (SQL), which is what the Query API uses

If I'm honest, I'm not 100% convinced that the concept of management domains and federated catalogs has been fully understood yet.

@@ -41,7 +41,7 @@ special [asset type](https://github.com/eclipse-edc/Connector/blob/main/docs/dev
was introduced for this purpose.

Every target node produces one `Catalog`, so in the end there is a `List<Catalog>` which contains all the assets that
are available in a particular dataspace.
are available in a particular dataspace. Like so, after the Federated Catalog Crawler retrieves all catalogs, an aggregation of all is made and one single root catalog is exposed, similar to a Catalog Server without CatalogAssets.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the crawler does not expose anything, least of all a root catalog.

@github-actions github-actions bot removed the stale label Dec 13, 2024
@rafaelmag110
Copy link
Contributor

@bmg13
Given the latest review from Paul, I'd suggest you to consider his points and start again from a fresh slate. I'd close this PR, spend some time understanding the concepts that are missing and then I'd get back with a new one.

@bmg13
Copy link
Contributor Author

bmg13 commented Dec 17, 2024

@bmg13 Given the latest review from Paul, I'd suggest you to consider his points and start again from a fresh slate. I'd close this PR, spend some time understanding the concepts that are missing and then I'd get back with a new one.

Following this feedback will close this pr and revisit in the future if opportunity arises.

@bmg13 bmg13 closed this Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants