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

Code Duplication in Cloud Plugins #2155

Open
kartg opened this issue Feb 17, 2022 · 7 comments
Open

Code Duplication in Cloud Plugins #2155

kartg opened this issue Feb 17, 2022 · 7 comments
Assignees
Labels
enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers hacktoberfest Global event that encourages people to contribute to open-source.

Comments

@kartg
Copy link
Member

kartg commented Feb 17, 2022

Is your feature request related to a problem? Please describe.
There seems to be duplication of logic in the code for Cloud Plugins. This is an opportunity for us to streamline the codebase and refactor these classes. This issue stems from these comments in #2096.

Describe the solution you'd like
Move common functionality to superclasses and have the Cloud Plugins inherit/extend these in platform-specific subclasses.

Describe alternatives you've considered
N/A

Additional context
N/A

@kartg kartg added enhancement Enhancement or improvement to existing feature or request untriaged labels Feb 17, 2022
@kartg kartg added the good first issue Good for newcomers label Feb 17, 2022
@willyborankin
Copy link
Contributor

willyborankin commented Feb 21, 2022

Small investigation I made.

Common code I found:
List of common classes and functions for proxy settings in all cloud plugins:

  • ProxySettings - could be a generic type, since all clients have they own ProxyType
  • *Settings classes - the function validateAndCreateProxySettings has the same code for all plugins
    Common functions:
  • all *Settings::getConfigValue and *Settings::getValue classes use the same logic to get settings

Common settings:

  • client
  • compress
  • readonly
  • chunk_size
  • base_path
  • all proxy stettings

Common class:

  • SocketAccess - it is the same for all plugins

I think it is possible to build high level abstraction for the plugin and re-use it in plugins.
I tried to do it here https://github.com/aiven/aiven-repositories-for-opensearch, the code is far from ideal, maybe you will find it usefull.

@willyborankin
Copy link
Contributor

@CEHENKLE and @kartg if you are still interested in refactoring cloud repositories. I can help with it. Wdyt?

@kartg
Copy link
Member Author

kartg commented Jun 3, 2022

@willyborankin we're definitely still interesting in refactoring these plugins. Would you be willing to attempt a first pass with one of the plugins, and we can iteratively refactor the rest?

@willyborankin
Copy link
Contributor

Sure will prepare PR during this week

@sourav25998
Copy link

Hello @kartg . This issue seems to be a nice start to my open source contribution journey. Can you please assign this to me?

@dblock
Copy link
Member

dblock commented Sep 13, 2022

@sourav25998 i don’t think we can use assignments on GH due to permissions, but go for it! Make a PR

@kartg
Copy link
Member Author

kartg commented Sep 14, 2022

@dblock 👆 😉

@owaiskazi19 owaiskazi19 added the hacktoberfest Global event that encourages people to contribute to open-source. label Oct 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers hacktoberfest Global event that encourages people to contribute to open-source.
Projects
None yet
Development

No branches or pull requests

7 participants
@willyborankin @dblock @owaiskazi19 @sourav25998 @CEHENKLE @kartg and others