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

Initial Commit #1

Merged
merged 2 commits into from
May 3, 2022
Merged

Initial Commit #1

merged 2 commits into from
May 3, 2022

Conversation

owaiskazi19
Copy link
Member

Signed-off-by: Owais Kazi [email protected]

Description

Added the SDK code.

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

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.

Signed-off-by: Owais Kazi <[email protected]>
@@ -2,3 +2,28 @@
This project has adopted the [Amazon Open Source Code of Conduct](https://aws.github.io/code-of-conduct).
For more information see the [Code of Conduct FAQ](https://aws.github.io/code-of-conduct-faq) or contact
[email protected] with any additional questions or comments.

Copy link
Member

Choose a reason for hiding this comment

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

@owaiskazi19 can you open an issue to add the following:

  • Code Owners
  • Maintainers.md

I've updated the branch protection rules to require a PR to merge to main.

Copy link
Member Author

@owaiskazi19 owaiskazi19 Apr 29, 2022

Choose a reason for hiding this comment

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

Sure. #2

import java.util.Locale;
import java.util.concurrent.atomic.AtomicBoolean;

public class Netty4Utils {
Copy link
Member

@saratvemulapalli saratvemulapalli Apr 29, 2022

Choose a reason for hiding this comment

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

Could you open an issue to expose few of these classes as maven artifacts for SDK to consume? (on OpenSearch) and link it to the SDK meta issue as things to do.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@joshpalis joshpalis left a comment

Choose a reason for hiding this comment

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

Everything looks good

Copy link
Member

@saratvemulapalli saratvemulapalli left a comment

Choose a reason for hiding this comment

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

Love this.
Dropped a couple of minor comments.

return getGenericGroup();
}

// Remove HTTP calls
Copy link
Member

Choose a reason for hiding this comment

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

Is this intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I added that comment to specify that HTTP calls are removed here from the SharedGroupFactory file present in OpenSearch.

import static java.util.Collections.emptySet;
import static org.opensearch.common.UUIDs.randomBase64UUID;

public class RunPlugin {
Copy link
Member

Choose a reason for hiding this comment

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

Rename it to ExtensionsRunner?
No more plugins :)


package transportservice;

import io.netty.buffer.*;
Copy link
Member

Choose a reason for hiding this comment

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

Please open an issue to fail our precommits when * imports are used.
take a look at OpenSearch.

Copy link
Member Author

Choose a reason for hiding this comment

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

#3

@saratvemulapalli saratvemulapalli merged commit d0c74b8 into opensearch-project:main May 3, 2022
DarshitChanpura added a commit to DarshitChanpura/opensearch-sdk-java that referenced this pull request Aug 14, 2023
dbwiddis pushed a commit that referenced this pull request Aug 21, 2023
* Refactor changes from core #1

Signed-off-by: Darshit Chanpura <[email protected]>

* Fix * imports

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes missed refactor change

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds import order to formatting.gradle

Signed-off-by: Darshit Chanpura <[email protected]>

---------

Signed-off-by: Darshit Chanpura <[email protected]>
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