CARTAvis / carta-backend

Source code repository for the backend component of CARTA, a new visualization tool designed for the ALMA, the VLA and the SKA pathfinders.
https://cartavis.github.io/
GNU General Public License v3.0
22 stars 11 forks source link

skip running casa_data_autoupdate if checking version #1308

Closed ajm-ska closed 10 months ago

ajm-ska commented 1 year ago

Description

At the moment if a user just wants to check the version number with --version or -v, the /usr/bin/carta script runs the casa_data_autoupdate script. This is not really necessary.

[ajm@almat1 ~]# carta --version
CARTA will use the default ephemerides and geodetic data.
4.0.0-rc.0

Similarly, it is not necessary to run for --help or -h

We could easily skip it with the following modification to /usr/bin/carta:

if [[ ! "$@" =~ (--version|-v|--help|-h) ]]; then
    if [ -x "$(command -v casa_data_autoupdate)" ]; then
        casa_data_autoupdate
    fi
fi

This is a very small PR, so maybe we don't really need an accompanying issue?

Checklist

confluence commented 1 year ago

I think that we need to use real option parsing for this (using string parsing for this is fragile, and it's hard to avoid false positives and negatives). That makes this non-trivial enough that I think we should delay it to the next release.

kswang1029 commented 12 months ago

@ajm-asiaa would you sync up this branch with dev please (15 commits behind)?

github-actions[bot] commented 12 months ago

Code Coverage

Package Line Rate Health
src.Cache 63%
src.DataStream 44%
src.FileList 67%
src.Frame 36%
src.HttpServer 42%
src.ImageData 28%
src.ImageFitter 83%
src.ImageGenerators 43%
src.ImageStats 72%
src.Logger 34%
src.Main 52%
src.Region 17%
src.Session 4%
src.Table 52%
src.ThreadingManager 67%
src.Timer 85%
src.Util 30%
Summary 32% (6023 / 18675)
kswang1029 commented 12 months ago

from @confluence

I think a complete test would be: 1) check that each of those options prevents the update from running; 2) check that --host and --verbosity don't; 3) check that putting the options first or last still works. Those were the problems we found before.

kswang1029 commented 11 months ago

Using the backend executable from my dev environment, I tried

./carta_backend --frontend_folder /Users/kswang/carta_build/carta-frontend/build --port 3002 --omp_threads 8 --debug_no_auth --no_browser --verbosity 5 --version

./carta_backend --version --frontend_folder /Users/kswang/carta_build/carta-frontend/build --port 3002 --omp_threads 8 --debug_no_auth --no_browser --verbosity 5

./carta_backend --help --frontend_folder /Users/kswang/carta_build/carta-frontend/build --port 3002 --omp_threads 8 --debug_no_auth --no_browser --verbosity 5

./carta_backend --frontend_folder /Users/kswang/carta_build/carta-frontend/build --port 3002 --omp_threads 8 --debug_no_auth --no_browser --verbosity 5 --help

./carta_backend --frontend_folder /Users/kswang/carta_build/carta-frontend/build --version --port 3002 --omp_threads 8 --debug_no_auth --no_browser --verbosity 5

./carta_backend --frontend_folder /Users/kswang/carta_build/carta-frontend/build --help --port 3002 --omp_threads 8 --debug_no_auth --no_browser --verbosity 5

and they all work fine.

Then I tried the macOS electron app with an alias pointed to /Applications/CARTA-17b983ea-f35e050e-2023-12-11.app/Contents/MacOS/CARTA-17b983ea-f35e050e-2023-12-11. The electron window will be launched instead of showing a version string in the terminal with I set --version. With v4.0 I see

carta --version CARTA will use the default ephemerides and geodetic data.

4.0.0

Then I tried the AppImage (carta-17b983ea-f35e050e-2023-12-11-x86_64.AppImage) on ubuntu with --help and --version flags. I saw in both cases, "CARTA will use the default ephemerides and geodetic data" string showed up in the terminal.

So, @ajm-asiaa, do you think what I saw from the macOS electron app and AppImage was due to the extra configurations apart from the carta_backend executable?

ajm-ska commented 11 months ago

@kswang1029 For the macOS Electron app, the electron window launched instead of only showing the version string in the terminal because it would require this PR to be merged in the carta-package repository: https://github.com/CARTAvis/carta-package/pull/14

I'm not sure why the "CARTA will use the default ephemerides and geodetic data." line is also showing up there because it should be running the "carta-backend/bin/run.sh" script. So I should double-check Auto App Assembler is picking up the correct branches.

I forgot that the AppImage has its own slightly longer startup script: https://github.com/CARTAvis/carta-package/blob/master/CARTA-AppImage-creator/AppRun I'll make a branch and PR on the carta-package repo for the AppImage startup script with similar modifications.

ajm-ska commented 11 months ago

I created and merged carta-package PR#15 and also went ahead and merged PR#14. They are just minor changes. I will create a new AppImage and macOS Electron app, and check the issue with the macOS Electron app.

ajm-ska commented 11 months ago

@kswang1029 All done. I think the macOS Electron app and AppImage appear to handle the --version flag properly now.

macOS Electron: https://carta.asiaa.sinica.edu.tw/downloads/CARTA-9a502ee9-f35e050e-2023-12-12-4.0.0-test-arm64.dmg alias carta=/Applications/CARTA-9a502ee9-f35e050e-2023-12-12.app/Contents/MacOS/CARTA-9a502ee9-f35e050e-2023-12-12

Appimage: https://carta.asiaa.sinica.edu.tw/downloads/carta-9a502ee9-f35e050e-2023-12-12-x86_64.AppImage