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

Extract the data-prepper-plugin-framework from data-prepper-core #4260

Merged
merged 3 commits into from
Mar 14, 2024

Conversation

graytaylor0
Copy link
Member

@graytaylor0 graytaylor0 commented Mar 11, 2024

Description

This change creates a high level module of data-prepper-plugin-framework and extracts the plugin framework code from data-prepper-core

It also

  • Creates an interface ExtensionConfiguration that DataPrepperConfiguration now implements
  • Moves DataPrepperDurationDeserializer and BytesDeserializer to data-prepper-pipeline-parser

Issues Resolved

Related to #2573

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

…module

Signed-off-by: Taylor Gray <tylgry@amazon.com>
…new module

Signed-off-by: Taylor Gray <tylgry@amazon.com>
kkondaka
kkondaka previously approved these changes Mar 14, 2024
@@ -0,0 +1,7 @@
package org.opensearch.dataprepper.plugin;
Copy link
Member

Choose a reason for hiding this comment

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

We should probably change this package to be something else. But, maybe in another PR.

@@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.dataprepper.parser;
package org.opensearch.dataprepper.pipeline.parser;
Copy link
Member

Choose a reason for hiding this comment

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

We should look to see if we can make this package protected. Perhaps in another PR. It might involve moving a few other classes to reside in the same package.

@@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.dataprepper.parser;
package org.opensearch.dataprepper.pipeline.parser;
Copy link
Member

Choose a reason for hiding this comment

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

We should look to see if we can make this package protected. Perhaps in another PR. It might involve moving a few other classes to reside in the same package.

@@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.dataprepper.plugin;
package org.opensearch.dataprepper.plugins;
Copy link
Member

Choose a reason for hiding this comment

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

I think this should remain in the old package. It uses package protected classes.

Also, we want to be sure that it does not interfere with loading the test plugins, which reside in .plugins.

The names are a little ambiguous. But, .plugin is the plugin framework and .plugins is the package for loading classes.

@@ -25,7 +25,7 @@
* <p><i>publicContext</i> is the root {@link ApplicationContext}</p>
*/
@Named
class PluginBeanFactoryProvider implements Provider<BeanFactory> {
public class PluginBeanFactoryProvider implements Provider<BeanFactory> {
Copy link
Member

Choose a reason for hiding this comment

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

Can we keep this package protected?

My initial suspicion is that this was caused by making DefaultPluginFactoryIT reside in .plugins.

Signed-off-by: Taylor Gray <tylgry@amazon.com>
Copy link
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

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

Thanks, this change is going to be a very helpful improvement.

@graytaylor0 graytaylor0 merged commit 7cbfb0a into opensearch-project:main Mar 14, 2024
69 of 74 checks passed
@graytaylor0 graytaylor0 deleted the PluginFramework branch March 14, 2024 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants