-
Notifications
You must be signed in to change notification settings - Fork 25
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
Add library to check release issues #589
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #589 +/- ##
============================================
- Coverage 84.61% 83.62% -0.99%
- Complexity 133 142 +9
============================================
Files 123 124 +1
Lines 780 843 +63
Branches 86 100 +14
============================================
+ Hits 660 705 +45
- Misses 33 41 +8
- Partials 87 97 +10 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Sayali Gaikawad <[email protected]>
Signed-off-by: Sayali Gaikawad <[email protected]>
Signed-off-by: Sayali Gaikawad <[email protected]>
Signed-off-by: Sayali Gaikawad <[email protected]>
Signed-off-by: Sayali Gaikawad <[email protected]>
f278751
to
c9dfa6c
Compare
@@ -58,7 +61,7 @@ class ReleaseMetricsData { | |||
], | |||
[ | |||
match_phrase: [ | |||
"component": "${component}" | |||
"component.keyword": "${component}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need this if component
field is of keyword
type.
Was there any change in results?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the type is text. This is the mapping:
"component": {
"type": "text",
"fields": {
"keyword": {
"type": "keyword",
"ignore_above": 256
}
}
},
Without keyword
the the results return everything that has opensearch in it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally the field should have been keyword type. I'm okay if mapping cannot be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right! I am just retrieving data from that index. @prudhvigodithi might be able to add more.
String action = args.action ?: 'check' | ||
|
||
// Parameter check | ||
if (!inputManifest || inputManifest.isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same boolean isNullOrEmpty(String str) { return (str == null || str.allWhitespace || str.isEmpty()) }
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also an array. Both manifests for OS and OSD can be supplied at the same time. While calling this method in the orchestrator we will only take version as an input and hardcode manifest file paths.
error "Invalid action '${action}'. Valid values: check, create" | ||
} | ||
|
||
def inputManifestYaml = readYaml(file: args.inputManifest[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why [0] index?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, just to get the version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! Since its an array just taking whichever is the first manifest and parsing it.
} | ||
echo("Components missing release issues: " + componentsMissingReleaseIssue) | ||
|
||
if (action == 'create' && !componentsMissingReleaseIssue.isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why check and create are two different actions, would it be better to check if the componentsMissingReleaseIssue list is not empty and just create the issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Synced up offline. This library is providing flexibility to check as well. However, the orchestrator will be hardcoding to check if required.
Signed-off-by: Sayali Gaikawad <[email protected]> (cherry picked from commit 533a9b1) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Add library to check release issues and trigger the GHA workflow if any are missing.
Issues Resolved
opensearch-project/opensearch-build#5332
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.