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: restructure extensions by component #1863

Merged
merged 1 commit into from
Aug 24, 2022
Merged

feat: restructure extensions by component #1863

merged 1 commit into from
Aug 24, 2022

Conversation

ndr-brt
Copy link
Member

@ndr-brt ndr-brt commented Aug 22, 2022

What this PR changes/adds

move all the extensions folders into the extensions folder, grouped by the component they are needed by:

extensions
├── common
│   ├── api
│   │   ├── api-core
│   │   └── observability
│   ├── auth
│   │   ├── auth-basic
│   │   └── auth-tokenbased
│   ├── aws
│   │   ├── aws-test
│   │   └── s3-core
│   ├── azure
│   │   ├── azure-eventgrid
│   │   ├── azure-resource-manager
│   │   ├── azure-test
│   │   ├── blob-core
│   │   └── cosmos-common
│   ├── configuration
│   │   └── filesystem-configuration
│   ├── events
│   │   └── cloudevents-http
│   ├── http
│   │   ├── jersey
│   │   ├── jersey-micrometer
│   │   ├── jetty
│   │   └── jetty-micrometer
│   ├── iam
│   │   ├── daps
│   │   ├── decentralized-identity
│   │   ├── iam-mock
│   │   └── oauth2
│   ├── junit
│   ├── micrometer
│   ├── monitor
│   │   └── jdk-logger-monitor
│   ├── sql
│   │   ├── common-sql
│   │   ├── lease-sql
│   │   └── pool
│   ├── transaction
│   │   ├── transaction-atomikos
│   │   └── transaction-local
│   └── vault
│       ├── azure-vault
│       ├── filesystem-vault
│       └── hashicorp-vault
├── control-plane
│   ├── api
│   │   └── data-management
│   ├── cosmos
│   │   ├── assetindex-cosmos
│   │   ├── contract-definition-store-cosmos
│   │   ├── contract-negotiation-store-cosmos
│   │   ├── control-plane-cosmos
│   │   ├── policy-store-cosmos
│   │   └── transfer-process-store-cosmos
│   ├── data-plane-transfer
│   │   ├── data-plane-transfer-client
│   │   └── data-plane-transfer-sync
│   ├── http-receiver
│   ├── provision
│   │   ├── blob-provision
│   │   ├── http-provisioner
│   │   └── s3-provision
│   └── sql
│       ├── asset-index-sql
│       ├── contract-definition-store-sql
│       ├── contract-negotiation-store-sql
│       ├── control-plane-sql
│       ├── policy-store-sql
│       └── transfer-process-store-sql
├── data-plane
│   ├── data-plane-api
│   ├── data-plane-azure-storage
│   ├── data-plane-data-factory
│   ├── data-plane-http
│   ├── data-plane-s3
│   └── integration-tests
├── data-plane-selector
│   ├── selector-api
│   └── selector-client
├── federated-catalog
│   └── cosmos
│       └── fcc-node-directory-cosmos
└── iam
    └── daps

Why it does that

To accomplish project structure review

Further notes

  • The extensions separation works well by component and then by context (e.g. control-plane/provision/ or common/vault).
  • Only for certain modules in the common subfolder seem like the division by "provider" (e.g. aws/s3-core or azure/blob-core) is the only appliable
  • Introduced BOM modules: control-plane-cosmos and control-plane-sql, they should be pretty self-explanatory and helpful for sure to build a control plain with a certain persistence system
  • azure-resource-manager seems to be used only in pair with data-plane-data-factory so it could be considered a data-plane extension, but wasn't sure so I let it into the common folder
  • blobstorage module was inlined because it was giving nothing more than the blob-provider
  • the extensions/common/iam folder would need a further look. iam could have its own spi and core module that register a mock/noop default IdentityService then the related extensions could override it. to be addressed in another issue eventually.
  • I was in doubt if jetty-micrometer and jersey-micrometer should be moved to a extensions/common/micrometer forlder instead of being included in the extensions/common/http. For the moment I left them in the latter.

Linked Issue(s)

Closes #1810

Checklist

  • added appropriate tests?
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • assigned appropriate label? (exclude from changelog with label no-changelog)
  • formatted title correctly? (take a look at the CONTRIBUTING and styleguide for details)

@ndr-brt ndr-brt added enhancement New feature or request refactoring Cleaning up code and dependencies labels Aug 22, 2022
@jimmarino
Copy link
Contributor

I like this layout. A few comments and suggestions:

  1. data-plane-selector should be under control-plane
  2. There is a bit of an inconsistency in control-plane where cosmos is its own root rather than part of a subsystem. Could a control-plane/store directory be introduced with cosmos and sql placed under that?
  3. Similarly, rename federated-catalog/cosmos to federated-catalog/store and include the cosmos extension directly in that folder since there is only one.
  4. Move the top level iam/daps module under common/daps (was this a to?)

@ndr-brt
Copy link
Member Author

ndr-brt commented Aug 23, 2022

I like this layout. A few comments and suggestions:

  1. data-plane-selector should be under control-plane

for what I understood, the data-plane-selector could be deployed as a component of its own, separated from the control-plane (as there are two DataPlaneSelectorClient implementation: Embedded and Remote), so I would let it in a separate folder, same is for data-plane, it can be deployed together as control-plane, but it is a different logic component.

  1. There is a bit of an inconsistency in control-plane where cosmos is its own root rather than part of a subsystem. Could a control-plane/store directory be introduced with cosmos and sql placed under that?
  2. Similarly, rename federated-catalog/cosmos to federated-catalog/store and include the cosmos extension directly in that folder since there is only one.

I was thinking the same, my first idea was "persistence", but "store" fits better and it is shorted, will address this.

  1. Move the top level iam/daps module under common/daps (was this a to?)

could make sense to have common/oauth2/daps? As we have an oauth2-spi it could make sense to have a folder representing the oauth2 extensions.

EDIT: thinking about the last one... do they fit well in the iam folder? like:

iam/decentralized-identity
iam/oauth2/oauth2-core
iam/oauth2/daps
iam/iam-mock

so the only thing to do could be to move daps into oauth2, as it is an oauth2 specialization

@jimmarino
Copy link
Contributor

I like this layout. A few comments and suggestions:

  1. data-plane-selector should be under control-plane

for what I understood, the data-plane-selector could be deployed as a component of its own, separated from the control-plane (as there are two DataPlaneSelectorClient implementation: Embedded and Remote), so I would let it in a separate folder, same is for data-plane, it can be deployed together as control-plane, but it is a different logic component.

  1. There is a bit of an inconsistency in control-plane where cosmos is its own root rather than part of a subsystem. Could a control-plane/store directory be introduced with cosmos and sql placed under that?
  2. Similarly, rename federated-catalog/cosmos to federated-catalog/store and include the cosmos extension directly in that folder since there is only one.

I was thinking the same, my first idea was "persistence", but "store" fits better and it is shorted, will address this.

  1. Move the top level iam/daps module under common/daps (was this a to?)

could make sense to have common/oauth2/daps? As we have an oauth2-spi it could make sense to have a folder representing the oauth2 extensions.

EDIT: thinking about the last one... do they fit well in the iam folder? like:

iam/decentralized-identity
iam/oauth2/oauth2-core
iam/oauth2/daps
iam/iam-mock

so the only thing to do could be to move daps into oauth2, as it is an oauth2 specialization

+1 on those. I'm also fine to have data-plane-selector top-level.

@ndr-brt ndr-brt merged commit 76a3825 into eclipse-edc:main Aug 24, 2022
@ndr-brt ndr-brt deleted the feature/1810-restructure-extensions branch August 24, 2022 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactoring Cleaning up code and dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Project structure: extensions
3 participants