Issue 613 java17 #622
Issue 613 java17 #622
Conversation
# Conflicts: # build.xml
|
Do the classpath values set in the Line 27 in 06fb27c |
Yes, they should. Thank you for catching this |
|
The classpath in scripts has been updated |
|
@TatianaBurek, could these changes, or a subset of them, also be applied to the main_v6.0 branch? I'm mainly interested in the changes that resolve the environment issues that are causing batch plots using Python execution to fail. |
@georgemccabe Yes, I can update the 'old' code with the changes for env vars. It would be a good idea to test it.
|
|
@TatianaBurek, if you can create a branch off of main_v6.0 to apply the changes, that would be great. I recently added a GitHub Actions workflow to create a development Docker image to use for testing, so I can create one from your branch. |
|
@TatianaBurek, the mv_batch.sh (in its original form) does not work, in spite of setting the PRE_LOAD_CHROME env to True. |
I tested new mv_batch.sh @bikegeek how do you test mv_batch.sh? |
I ran with Rscript and python as execution type and get permission issues with loading chrome, in spite of setting PRE_LOAD_CHROME=True: PermissionError: [Errno 13] Permission denied: '/d3/projects/METViewer/miniforge3/envs/METviewer_py3.12/lib/python3.12/site-packages/choreographer/cli/browser_exe/chrome.zip' |
@bikegeek Where do you run it? On dakota? Which user do you use? |
|
@TatianaBurek |
@bikegeek Do you mean mv_batch.sh? |
|
I was also testing the mv_load.sh script, in addition to mv_batch.sh. The
mv_batch.sh is the one with the chrome permission errors. I don't know why
it is attempting to download and save chrome when the PRE_LOAD_CHROME env
var is set to True (and verified via a printenv command)
…On Tue, Oct 14, 2025 at 2:06 PM Tatiana Burek ***@***.***> wrote:
*TatianaBurek* left a comment (dtcenter/METviewer#622)
<#622 (comment)>
@TatianaBurek <https://github.com/TatianaBurek> I am running it on
'dakota' as me. The mv_load.sh script is working and I'm generating line
plots successfully.
@bikegeek <https://github.com/bikegeek> Do you mean mv_batch.sh?
—
Reply to this email directly, view it on GitHub
<#622 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4UJHV3F54P2XSFKMO42GD3XVJTDAVCNFSM6AAAAACJE6WCYGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIMBTGQZDQMBYHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@bikegeek I just tested /d3/projects/METViewer/src_dev/apps/METviewer//bin/mv_batch.sh on dakota as user 'tatiana'. |
|
I'll try it again without setting any environment vars.
…On Tue, Oct 14, 2025 at 2:22 PM Tatiana Burek ***@***.***> wrote:
*TatianaBurek* left a comment (dtcenter/METviewer#622)
<#622 (comment)>
@bikegeek <https://github.com/bikegeek> I just tested
/d3/projects/METViewer/src_dev/apps/METviewer//bin/mv_batch.sh on dakota as
user 'tatiana'.
I do not set any environment variables. The script run successfully and
generated an image
—
Reply to this email directly, view it on GitHub
<#622 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA4UJHTH7A7OUDSDU6KBEWT3XVLQRAVCNFSM6AAAAACJE6WCYGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIMBTGQ4DMOBUGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@TatianaBurek, I see in the issue that these changes update to use Tomcat 10. The METbaseimage used to create the METviewer Docker images downloads Tomcat 9.0.108 (see https://github.com/dtcenter/METbaseimage/blob/73c1a333180b93eebd95627ae6180481cef2ee6f/Dockerfile.metviewer#L11-L14). Will these changes work with Tomcat 9 or will we need to update the base image to obtain Tomcat 10? |
|
The new code works only with Tomcat 10. |
|
UPDATE: I meant to say v6.2 instead of v7.0. I forgot METviewer 6.2.0-rc1 has not been created yet. OK, thanks for confirming. I think this means that I will create a new version of the METbaseimage that will work with these changes that will be used to create METviewer 6.2. I think this also means that only the changes that resolve the environment issues should be applied to the Does all that sound good/correct?
|
|
@georgemccabe The Java version also needs to be changed in the METbaseimage to Java 17 together with the Tomcat version. |
|
@TatianaBurek, the base images used for METviewer going back to main_v6.0 install If the env var fixes resolve the issues with making batch plots using Python, then they should be applied to both main_v6.0 and main_v6.1. |
|
I put the fixes to the corresponding brunches for 6_0 and 6_1 |
|
TODO: In Update section of the release notes, instruct users to ensure that all users (ugo) have execute permission to the python3.12/site-packages/choreographer/cli/browser_exe directory via chmod -R +x path/to/choreographer/cli/browser_exe directory AND also have permissions set to their home directory (from the host where the mv_batch command is run) to allow creation of the .configure directory |
update MET_BASE_TAG to 3.5-latest to use Tomcat 10
Pull Request Testing
Describe testing already performed for these changes:
Lightly tested on dakota (only with Python since R is not available)
Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
Create different plots, download and upload XML files
Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes or No]
No
Do these changes include sufficient testing updates? [Yes or No]
No
Will this PR result in changes to the test suite? [Yes or No]
If yes, describe the new output and/or changes to the existing output:
No
Do these changes introduce new SonarQube findings? [Yes or No]
If yes, please describe:
Please complete this pull request review by [Fill in date].
Pull Request Checklist
See the METplus Workflow for details.
Select: Reviewer(s) and Development issue
Select: Milestone as the version that will include these changes
Select: Coordinated METplus-X.Y Support project for bugfix releases or METviewer-X.Y.Z Development project for official releases