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

Support ByteCount in plugin parser #3191

Closed
dlvenable opened this issue Aug 17, 2023 · 4 comments · Fixed by #4003 · May be fixed by #3326
Closed

Support ByteCount in plugin parser #3191

dlvenable opened this issue Aug 17, 2023 · 4 comments · Fixed by #4003 · May be fixed by #3326
Assignees
Labels
good first issue Good for newcomers maintenance Issues to help maintain the project, such as improving builds, testing, etc.
Milestone

Comments

@dlvenable
Copy link
Member

Is your feature request related to a problem? Please describe.

Plugin authors must use strings and manually convert to use ByteCount in their configuration files.

Describe the solution you'd like

Support ByteCount deserialization in the plugin parser. This would be similar to the data-prepper-config.yaml which supports it already. And it would give similar to behavior to the duration deserializer.

Additional context

Needs to be added here:

simpleModule.addDeserializer(Duration.class, new DataPrepperDurationDeserializer());

Compare with:

final SimpleModule simpleModule = new SimpleModule()
.addDeserializer(Duration.class, new DataPrepperDurationDeserializer())
.addDeserializer(ByteCount.class, new ByteCountDeserializer());

@dlvenable dlvenable added good first issue Good for newcomers maintenance Issues to help maintain the project, such as improving builds, testing, etc. labels Aug 17, 2023
@Durdush
Copy link

Durdush commented Aug 20, 2023

Hi @dlvenable , Could you please assign me this.

@dlvenable
Copy link
Member Author

@Durdush , Thank you for your interest in working this! Do you have any questions about how to accomplish it? I believe I have some information in the description to get started on it.

@dlvenable dlvenable linked a pull request Sep 13, 2023 that will close this issue
4 tasks
@GumpacG
Copy link
Contributor

GumpacG commented Jan 23, 2024

Hi, is this issue still relevant? I can submit a PR to address this.
Seems that it supported in pluginConfigObjectMapper but not in extensionPluginConfigObjectMapper.

ObjectMapper pluginConfigObjectMapper(final VariableExpander variableExpander) {
final SimpleModule simpleModule = new SimpleModule();
simpleModule.addDeserializer(Duration.class, new DataPrepperDurationDeserializer());
simpleModule.addDeserializer(ByteCount.class, new ByteCountDeserializer());

ObjectMapper extensionPluginConfigObjectMapper() {
final SimpleModule simpleModule = new SimpleModule();
simpleModule.addDeserializer(Duration.class, new DataPrepperDurationDeserializer());
return new ObjectMapper()
.setPropertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
.registerModule(simpleModule);
}

I can use #3326 as reference since it's been a while since it's been active.

@dlvenable
Copy link
Member Author

@GumpacG,

This was partially resolved in pluginConfigObjectMapper as you noted. However, this will not be available until 2.7.0.

https://github.com/opensearch-project/data-prepper/pull/3958/files#diff-ada28b8b3eeeba5a7e1e322dcff5b6b90a3a79c174f011609f826972f1f302dcR38

I don't think we resolved this in extensionPluginConfigObjectMapper yet. Feel free to create a PR to finish it up. Thanks!

@dlvenable dlvenable added this to the v2.7 milestone Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers maintenance Issues to help maintain the project, such as improving builds, testing, etc.
Projects
4 participants