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.

Sign up for GitHub

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

Jump to bottom

Updating Developer guide #84

Merged
merged 7 commits into from
Aug 15, 2022
Merged

Updating Developer guide #84

merged 7 commits into from
Aug 15, 2022

Conversation

vibrantvarun
Copy link
Member

@vibrantvarun vibrantvarun commented Aug 11, 2022 •
edited
Loading

Signed-off-by: Varun Jain varunudr@amazon.com

Description

Describe what this change achieves.
Updating Developer guide with steps to run artifact

Issues Resolved

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

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.

@vibrantvarun
Updating Developer guide
0ae97e5 
Signed-off-by: Varun Jain <varunudr@amazon.com>
@vibrantvarun vibrantvarun added the documentation Improvements or additions to documentation label Aug 11, 2022
@vibrantvarun vibrantvarun requested a review from a team August 11, 2022 06:37
@vibrantvarun vibrantvarun self-assigned this Aug 11, 2022
owaiskazi19
owaiskazi19 reviewed Aug 11, 2022
View reviewed changes
DEVELOPER_GUIDE.md Outdated

```
./gradlew clean
./gradlew build
Copy link
Member

Choose a reason for hiding this comment

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

./gradlew clean && ./gradlew build

Copy link
Member Author

Choose a reason for hiding this comment

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

Commited the changes and also did the rebase with main

@owaiskazi19
Copy link
Member

owaiskazi19 commented Aug 11, 2022 •
edited
Loading

@vibrantvarun please add description and the issue resolved. Needs rebase with the latest main

@joshpalis joshpalis self-requested a review August 11, 2022 17:29
@vibrantvarun
Copy link
Member Author

@vibrantvarun please add description and the issue resolved. Needs rebase with the latest main

@vibrantvarun vibrantvarun reopened this Aug 11, 2022
joshpalis
joshpalis reviewed Aug 11, 2022
View reviewed changes
DEVELOPER_GUIDE.md
./gradlew clean
./gradlew build
```
Once the tar ball is generated navigate to /src/test/resources and look for extension.yml. Create one if not present
Copy link
Member

Choose a reason for hiding this comment

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

Running the artifact would look for the extension.yml file within the opensearch-sdk/build/distributions/opensearch-sdk-1.0.0-SNAPSHOT/src/test/resources/ instead of the opensearch-sdk/src/test/resources directory. This will cause the following error:
Exception in thread "main" java.io.FileNotFoundException: src/test/resources/extension.yml (No such file or directory) at java.base/java.io.FileInputStream.open0(Native Method) at java.base/java.io.FileInputStream.open(FileInputStream.java:212) at java.base/java.io.FileInputStream.<init>(FileInputStream.java:154) at com.fasterxml.jackson.dataformat.yaml.YAMLFactory.createParser(YAMLFactory.java:354) at com.fasterxml.jackson.dataformat.yaml.YAMLFactory.createParser(YAMLFactory.java:15) at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3494) at org.opensearch.sdk.ExtensionsRunner.readExtensionSettings(ExtensionsRunner.java:90) at org.opensearch.sdk.ExtensionsRunner.<init>(ExtensionsRunner.java:67) at org.opensearch.sdk.ExtensionsRunner.main(ExtensionsRunner.java:386)

Perhaps we should add this as a note and remind developers to create the extension.yml file within the correct directory prior to running the binary

Copy link
Member

Choose a reason for hiding this comment

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

Create #86 to address this

Copy link
Member

Choose a reason for hiding this comment

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

Let's resolve #86 before merging this then. @joshpalis @vibrantvarun WDYT?

@vibrantvarun
Updating Developer Guide
27d7848 
Signed-off-by: Varun Jain <varunudr@amazon.com>
joshpalis
joshpalis previously approved these changes Aug 11, 2022
View reviewed changes
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.

LGTM!

owaiskazi19
owaiskazi19 reviewed Aug 12, 2022
View reviewed changes
DEVELOPER_GUIDE.md Outdated
```
tar -xvf opensearch-sdk-1.0.0-SNAPSHOT.tar
```
For now, create the extension.yml file within opensearch-sdk/build/distributions/opensearch-sdk-1.0.0-SNAPSHOT/src/test/resources/ and then run ./bin/opensearch-sdk to start opensearch
Copy link
Member

Choose a reason for hiding this comment

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

Move this above here

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if the above link works. I meant here

Create one if not present (For now, create the extension.yml file within  opensearch-sdk/build/distributions/opensearch-sdk-1.0.0-SNAPSHOT/src/test/resources/  and then run ./bin/opensearch-sdk to start 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.

Once the user untar the artifact then this step has to be performed, therefore i have written it after that step

Copy link
Member

@owaiskazi19 owaiskazi19 Aug 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Also, let's change it to

 For now until https://github.com/opensearch-project/opensearch-sdk/issues/86 is resolved, create....

Copy link
Member

Choose a reason for hiding this comment

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

Let's add details of what should be present in extensions.yml file

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@vibrantvarun
Update Developer Guide
8dfbb18 
Signed-off-by: Varun Jain <varunudr@amazon.com>
joshpalis
joshpalis previously approved these changes Aug 12, 2022
View reviewed changes
owaiskazi19
owaiskazi19 reviewed Aug 12, 2022
View reviewed changes
DEVELOPER_GUIDE.md
tar -xvf opensearch-sdk-1.0.0-SNAPSHOT.tar
```
For now, navigate to opensearch-sdk/build/distributions/opensearch-sdk-1.0.0-SNAPSHOT/
- Check if src folder exists in using `ls`.
Copy link
Member

@owaiskazi19 owaiskazi19 Aug 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Are we not sure if src folder will exist? Did we have any case where it didn't actually?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah we have to create src folder

Copy link
Member Author

@vibrantvarun vibrantvarun Aug 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Once #52 get resolved, there will no need to create src folder as location of extension.yml file will be finalized.

@vibrantvarun
Update Developer Guide
22386ee 
Signed-off-by: Varun Jain <varunudr@amazon.com>
@vibrantvarun
Update Developer Guide
ca11bb8 
Signed-off-by: Varun Jain <varunudr@amazon.com>
@vibrantvarun vibrantvarun merged commit 0727269 into opensearch-project:main Aug 15, 2022
@vibrantvarun vibrantvarun deleted the Update_Developer_Guide_Issue_11 branch August 15, 2022 17:48
kokibas pushed a commit to kokibas/opensearch-sdk-java that referenced this pull request Mar 17, 2023
@vibrantvarun @kokibas
Updating Developer guide (opensearch-project#84)
35ec7a0 
* Updating Developer guide

Signed-off-by: Varun Jain <varunudr@amazon.com>

* Updating Developer Guide

Signed-off-by: Varun Jain <varunudr@amazon.com>

* Updating Developer Guide

Signed-off-by: Varun Jain <varunudr@amazon.com>

* Update Developer Guide

Signed-off-by: Varun Jain <varunudr@amazon.com>

* Update Developer Guide

Signed-off-by: Varun Jain <varunudr@amazon.com>

* Update Developer Guide

Signed-off-by: Varun Jain <varunudr@amazon.com>

Signed-off-by: Varun Jain <varunudr@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saratvemulapalli saratvemulapalli saratvemulapalli approved these changes

@joshpalis joshpalis joshpalis approved these changes

@owaiskazi19 owaiskazi19 Awaiting requested review from owaiskazi19 owaiskazi19 is a code owner

Assignees

@vibrantvarun vibrantvarun

Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vibrantvarun @owaiskazi19 @saratvemulapalli @joshpalis