-
-
Notifications
You must be signed in to change notification settings - Fork 314
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
Script Update to include latest Exclude files for JDK versions #4897
Conversation
Here are the Grinder test runs |
67d2a5b
to
5e143d4
Compare
} | ||
|
||
getExcludeFiles(){ | ||
github_last_modified=$(curl -s -H "Authorization: token $GIT_TOKEN" "https://api.github.ibm.com/repos/runtimes/JCK$JCK_VERSION-unzipped/commits?path=excludes/jck$JCK_FOLDER_SUFFIX.jtx" | awk -F'"date":' '{if(NF>1){gsub(/[",]/,"",$2); print $2; exit}}') |
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.
- Please do not hardcode a specific repo
- It is not limited to one
jck$JCK_FOLDER_SUFFIX.jtx
. There could be other files.
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.
In artifactory material we have only .jtx file in /exc
. So we are comparing last modified of .jtx file from Git repo with artifactory material to decide if update is needed or not.
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.
The script is also used by other vendors (e.g. Adoptium), not only ibm, so the link repo cannot be hardcoded.
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.
Created a parameter GIT_EXCLUDE_API_URL
to provide API url in JCK_Sync job.
JCK_Sync#928
28065f6
to
5aabfa5
Compare
JCK_Sync #924 -- Latest exclude file not available JCK_Sync #921 -- Latest exclude file available JCK_Sync #925 -- No Exclude file present in Artifactory |
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.
Also, when checking this pipeline, the result of 924 and 925 are red, but if the material is already up to date the job should be green.
} | ||
|
||
getExcludeFiles(){ | ||
github_last_modified=$(curl -s -H "Authorization: token $GIT_TOKEN" "https://api.github.ibm.com/repos/runtimes/JCK$JCK_VERSION-unzipped/commits?path=excludes/jck$JCK_FOLDER_SUFFIX.jtx" | awk -F'"date":' '{if(NF>1){gsub(/[",]/,"",$2); print $2; exit}}') |
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.
The script is also used by other vendors (e.g. Adoptium), not only ibm, so the link repo cannot be hardcoded.
80b6a9a
to
a04569c
Compare
echo " JCK$JCK_VERSION $JCK_WITHOUT_BACKSLASH is latest and not in the repo $GIT_URL... Please proceed with download" | ||
get_JAVA_SDK | ||
else | ||
if [ $result -ne 0 ]; then |
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.
In this PR, result
and exclude_files
are global variables and they are used in multiple places. And $result != 0
means we need to update the test material. Similarly, $exclude_files != 0
means we need to update the exclude files. I find it isn't very clear.
Please define boolean parameters isTestMaterialUpdateNeeded
and isTestExcludeUpdateNeeded
instead.
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.
Using boolean parameters isTestMaterialUpdateNeeded
and isTestExcludeUpdateNeeded
.
isTestMaterialUpdateNeeded == 0
means we need to update the test material. Using 0
as true and 1
as false.
fi | ||
} | ||
|
||
#install Java | ||
get_JAVA_SDK(){ | ||
getJavaSDK() { |
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 misleading. The function does get SDK or install java. It just tries to set JAVA_SDK_PATH=$WORKSPACE/../../../../jdkbinary/j2sdk-image if JAVA_HOME does not exist. And JAVA_HOME is a user-provided value.
JAVA_HOME="$1"; shift;; |
Please remove this function completely. Instead of
JAVA_HOME="$1"; shift;; |
$WORKSPACE/../../../../jdkbinary/j2sdk-image
.Also remove L282 to 287, change it to
$JAVA_SDK_PATH/bin/java -jar $f -install shell_scripts -o $WORKSPACE/unpackjck
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.
Changes done as per review. Removed get_JAVA_SDK().
$JAVA_SDK_PATH
is the default unless user provides value.
JCK_Sync #939 -- Latest exclude file not available JCK_Sync #937 -- Latest exclude file available JCK_Sync #940 -- No Exclude file present in Artifactory |
538ebbe
to
ab27ca8
Compare
6380429
to
9ef945d
Compare
Until further notice PR is moved to Draft. |
e660083
to
8b930b0
Compare
Update Exclude files - Update script to include latest exclude files from artifactory for JDK versions Signed-off-by: Kapil Powar <[email protected]>
8b930b0
to
58bd207
Compare
Moved to internal repo. Closing it. |
runtimes/backlog/issues/1241
CC: @llxia