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

[Feature/extensions] Read from extensions.yml #3381

Merged
merged 7 commits into from
May 18, 2022
Merged

[Feature/extensions] Read from extensions.yml #3381

merged 7 commits into from
May 18, 2022

Conversation

ryanbogan
Copy link
Member

Description

Adds method and structure to read from extensions.yml file

Issues Resolved

3084

Check List

  • [ X] New functionality includes testing.
    • [X ] All tests pass
  • [ X] Commits are signed per the DCO using --signoff

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.

ryanbogan added 3 commits May 16, 2022 18:13
@ryanbogan
Fixed git errors
5802552 
Signed-off-by: Ryan Bogan <rbogan@amazon.com>
@ryanbogan
Added ExtensionsSettings file and corresponding method in ExtensionsO…
e876318 
…rchestrator to read from extensions.yml

Signed-off-by: Ryan Bogan <rbogan@amazon.com>
@ryanbogan
Removed unnecessary files
873a5fe 
Signed-off-by: Ryan Bogan <rbogan@amazon.com>
@ryanbogan ryanbogan requested review from a team and reta as code owners May 18, 2022 18:33
@ryanbogan ryanbogan changed the title Read from extensions.yml [Feature/extensions] Read from extensions.yml May 18, 2022
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 873a5fe
Log 5440

Reports 5440

@ryanbogan
Fixed lint errors
e620b3a 
Signed-off-by: Ryan Bogan <rbogan@amazon.com>
owaiskazi19
owaiskazi19 reviewed May 18, 2022
View reviewed changes
server/build.gradle Outdated
@@ -76,6 +76,7 @@ if (!isEclipse) {
}
}


Copy link
Member

Choose a reason for hiding this comment

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

nit: Remove extra line

owaiskazi19
owaiskazi19 reviewed May 18, 2022
View reviewed changes
server/src/main/java/org/opensearch/extensions/ExtensionsOrchestrator.java
@@ -93,6 +98,32 @@ private void extensionsDiscovery() throws IOException {
for (final Path plugin : PluginsService.findPluginDirs(extensionsPath)) {
try {
PluginInfo pluginInfo = PluginInfo.readFromProperties(plugin);

// TODO: Add unit tests for commented section below
Copy link
Member

@owaiskazi19 owaiskazi19 May 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Do we have an issue for unit tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's my next task. I'll create one once I merge this PR

owaiskazi19
owaiskazi19 reviewed May 18, 2022
View reviewed changes
server/src/test/java/org/opensearch/extensions/ExtensionsOrchestratorTests.java

assertNotNull(extensions);
assertEquals("firstExtension", extensions.getExtensions().get(0).getName());
assertEquals("9301", extensions.getExtensions().get(1).getPort());
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 add more assert statements for hostName, hostAddress, version and the reset

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure e620b3a
Log 5441

Reports 5441

@ryanbogan
Added more asserts to tests and removed unnecessary dependencies
5a52734 
Signed-off-by: Ryan Bogan <rbogan@amazon.com>
@ryanbogan ryanbogan requested review from owaiskazi19 and removed request for joshpalis May 18, 2022 20:43
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 5a52734
Log 5442

Reports 5442

@saratvemulapalli
Copy link
Member 

-----------
* What went wrong:
Execution failed for task ':plugins:repository-azure:yamlRestTest'.
> There were failing tests. See the report at: file:///var/CITOOL/workflow/OpenSearch_CI/PR_Checks/Gradle_Check/search/plugins/repository-azure/build/reports/tests/yamlRestTest/index.html

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
==============================================================================

2: Task failed with an exception.
-----------
* What went wrong:
Execution failed for task ':server:test'.
> There were failing tests. See the report at: file:///var/CITOOL/workflow/OpenSearch_CI/PR_Checks/Gradle_Check/search/server/build/reports/tests/test/index.html

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
==============================================================================

Server tests are expected to fail due to #2772
But plugin tests are failing too, could you take a look.

ryanbogan added 2 commits May 18, 2022 21:43
@ryanbogan
Fixed gradle check errors
30048df 
Signed-off-by: Ryan Bogan <rbogan@amazon.com>
@ryanbogan
Removed unused import
5c0bbf2 
Signed-off-by: Ryan Bogan <rbogan@amazon.com>
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 30048df
Log 5455

Reports 5455

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 5c0bbf2
Log 5458

Reports 5458

@ryanbogan ryanbogan merged commit 3b58b05 into opensearch-project:feature/extensions May 18, 2022
@ryanbogan ryanbogan mentioned this pull request Nov 17, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshpalis joshpalis joshpalis approved these changes

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

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

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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ryanbogan @opensearch-ci-bot @saratvemulapalli @owaiskazi19 @joshpalis