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.

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

FITS visualization plugin is missing aladin lite js files in usegalaxy.eu #194

Open
francoismg opened this issue Sep 29, 2023 · 58 comments
Open

Comments

@francoismg
Copy link

Hi, I'm not sure if this is the right place to report a bug about the usegalaxy.eu galaxy instance, please tell me where to report issues if it is not

Describe the bug
When trying to use the FITS visualization plugin in galaxy.eu with a .fits file the fits viewer is not loaded because of missing js files (aladin-lite js file https://aladin.cds.unistra.fr/).

Those files are supposed to be located in the "dist" folder of the plugin (copied during plugin build) and are loaded from an npm package (https://www.npmjs.com/package/aladin-lite-galaxy)

Galaxy Version and/or server at which you observed the bug
Galaxy Version: usegalaxy.eu current version

To Reproduce
Steps to reproduce the behavior:

  1. Download a fits file to the galaxy instance (one is available in the test-data folder of galaxy) and go to "Visualize" to
    select the FITS viewer plugin
  2. Use your browser dev tools console to see the error about the missing js files

Expected behavior
User should be presented with an aladin-lite instance displaying a space view and the selected fits file

Additional context
We have been able to build the plugin locally by forcing a plugin rebuild and we have also been able to install the aladin-lite-galaxy package through npm so everything looks fine on our end

@volodymyrss
Copy link

@bgruening we are not sure if this is for the galaxy code, bit it is where the functionality was contributed it is possible that something needs to be improved in the contribution to make the deployment more reliable.

@bgruening
Copy link
Member

I see this here:

(venv) galaxy@sn06:~/server$ tree static/plugins/visualizations/fits_image_viewer/static/
static/plugins/visualizations/fits_image_viewer/static/
├── dist
│   ├── 049a5201c34949755ec5.module.wasm
│   ├── 642.aladin.js
│   ├── 7538bf853f9b14029f986310747a9aad.png
│   ├── aladin.js
│   ├── aladin.js.LICENSE.tx t
│   ├── ccdb93d24585bd08a4261722b95e9e3d.png
│   └── package.json
├── logo.png
├── plugin_build_hash.txt
├── script.js
├── script.js.map
└── style.css

1 directory, 12 files

No aladin-lite js at all :(

@bgruening
Copy link
Member

I copied the files now to ~/server/static/plugins/visualizations/fits_image_viewer/static/dist/aladin-lite-galaxy now the JS file can be loaded, no 404. But it still does not work. Can you maybe have a look.
I have currently no idea why it should not work for us but somewhere else. All other Vis are also working on EU isn't it?

It looks like the files are copied to the wrong directory.

@bgruening
Copy link
Member

Any idea what Uncaught (in promise) TypeError: WebAssembly: Response has unsupported MIME type 'application/octet-stream' expected 'application/wasm' is?

@francoismg
Copy link
Author

francoismg commented Sep 29, 2023

I copied the files now to ~/server/static/plugins/visualizations/fits_image_viewer/static/dist/aladin-lite-galaxy now the JS file can be loaded, no 404. But it still does not work. Can you maybe have a look. I have currently no idea why it should not work for us but somewhere else. All other Vis are also working on EU isn't it?

It looks like the files are copied to the wrong directory.

Thanks, I will take a look.

Not sure about the other Vis i'm trying to download some fastq files so I can test other plugins

@bgruening
Copy link
Member

Oh don't do that :)

..../projects/code/galaxy/lib/galaxy/datatypes/test/5e5z.pdb and related file are all valid test files for various datatypes that you can use.

The file above I can visualise:

grafik

@bgruening
Copy link
Member

I think this is our second problem.

Uncaught (in promise) TypeError: WebAssembly: Response has unsupported MIME type 'application/octet-stream' expected 'application/wasm'

The MIME type is wrong for whatever reason :(
Unfortunately, this is really not my area of expertise :(

@bgruening
Copy link
Member

Ok, it works after I told nginx what wasm is.

I don't understand why this has worked before and now I need this hack.

@bgruening
Copy link
Member

For reference I needed to add:

application/wasm                                 wasm;

To /etc/nginx/mime.types.

This is currently not in our playbooks so it will stop working tomorrow after redeployment. I hope our real admins can add this to the playbooks on Monday.

However, there is still the 1st problem that the js files end-up in the wrong directory.

@francoismg
Copy link
Author

Oh don't do that :)

..../projects/code/galaxy/lib/galaxy/datatypes/test/5e5z.pdb and related file are all valid test files for various datatypes that you can use.

The file above I can visualise:

grafik

yes didn't think about the test files thanks :)

Great thanks a lot, but yea it seems weird that it would start to fail all of a sudden. Apparently it's not an uncommon problem from what I found but I don't know why it was working before.

Will look into the directory thing, it's a simple cp from node_modules so don't know what could be the problem but will look into it and get back to you

Thanks again

@bgruening
Copy link
Member

Your welcome!

@dsavchenko
Copy link

I tried it locally by running galaxy release_23.1 with run.sh with default configuration

Also needed to create static/plugins/visualizations/fits_image_viewer/static/dist/aladin-lite-galaxy directory and copy files there from static/plugins/visualizations/fits_image_viewer/static/dist to make it work

@francoismg
Copy link
Author

I tried it locally by running galaxy release_23.1 with run.sh with default configuration

Also needed to create static/plugins/visualizations/fits_image_viewer/static/dist/aladin-lite-galaxy directory and copy files there from static/plugins/visualizations/fits_image_viewer/static/dist to make it work

ok so latest dev has the same problem then? was working for me but probably I don't have the latest release. Will be easier to debug if testable locally

@francoismg
Copy link
Author

I tried locally with a previous version and a fresh install from latest dev but the plugin works fine for me even with the files on the wrong folder in latest dev.

I changed the build script so it will only copy folder content inside the correct directory and it seems to work too so maybe that could be an easy fix even without knowing the root cause

@volodymyrss
Copy link

So I still see this
image

Also there is this, but maybe it's an effect of another issue, not sure where does A come from, @francoismg ?

image

@francoismg
Copy link
Author

francoismg commented Oct 4, 2023

Yea the A is not defined error is triggered by the missing files it's not an independent error.

I guess the sys admins didn't had time to add the fix to the instance playbook. Basically once the new mime type is added to nginx the plugin should work again since bjorn moved the files to the right directory.

Like I said on monday I can't really reproduce the bug locally since the plugin is working fine for me with latest dev even when the aladin lite files are also copied at the root of the dist folder. But once I had removed the files from static/plugins.... and config/plugins.... and forced a new build the files were only copied to the right folder (with the original build script version and also with the workaround version which created the folder directly and copied the files into it after)

@bgruening
Copy link
Member

I confirm that we have no moved the nginx change into the playbook yet, we will try to do this next week.

@volodymyrss
Copy link

@dsavchenko finds that locally it also fails the same way sometimes. So maybe we can improve something if @dsavchenko explains a bit in detail?

@francoismg suggests to add selenium tests if it is feasible?
Also it is possible to use CDS CDN as a fall-back

@volodymyrss
Copy link

@francoismg so we try to improve this now? Adding tests, fallback, etc?

@francoismg
Copy link
Author

@francoismg so we try to improve this now? Adding tests, fallback, etc?

yes was working on it right now

will add a workaround to the mime type issue by relying first on the cdn version so we don't rely on the server configuration and then a fallback on the local version

will also update the build script so the folder is created by the script instead of copied which should correct the missing folder issue

and then look at the feasability to add selenium test on the plugin so it is tested in the ci/cd process

I will push the fix to the epfl galaxy fork so you and denys can test it since apparently the bug behavior differs on local installs

@francoismg
Copy link
Author

fix is available for test in this branch :

https://github.com/esg-epfl-apc/galaxy/tree/fix-visualization-plugin

@volodymyrss @dsavchenko

@volodymyrss
Copy link

fix is available for test in this branch :

https://github.com/esg-epfl-apc/galaxy/tree/fix-visualization-plugin

@volodymyrss @dsavchenko

Thanks! Please make a PR.

@dsavchenko
Copy link

@francoismg now I get 404 for script.js while starting galaxy locally with ./run.sh
(it's referenced as http://127.0.0.1:8080/static/plugins/visualizations/fits_image_viewer/static/script.js)

@francoismg
Copy link
Author

@francoismg now I get 404 for script.js while starting galaxy locally with ./run.sh (it's referenced as http://127.0.0.1:8080/static/plugins/visualizations/fits_image_viewer/static/script.js)

ok thanks, that's the correct location for script.js so it looks like it wasn't moved from the plugin static folder to the main static one, could you check if the plugin static folder /config/plugins/visualizations/fits_image_viewer/static contains script.js please?

@dsavchenko
Copy link

could you check if the plugin static folder /config/plugins/visualizations/fits_image_viewer/static contains script.js please?

It's not there. Only logo.png and style.css are there

@francoismg
Copy link
Author

could you check if the plugin static folder /config/plugins/visualizations/fits_image_viewer/static contains script.js please?

It's not there. Only logo.png and style.css are there

ok so no plugin_build_hash.txt too? When running run.sh do you see a specific error message during plugin build?

@dsavchenko
Copy link

I've just made a commit esg-epfl-apc@cf6c980 that seems to fix the issue for me

@francoismg
Copy link
Author

I've just made a commit esg-epfl-apc@cf6c980 that seems to fix the issue for me

ok great thanks, weird that suddenly you have to force the folder creation but I tested it and it has no side effects so we can do it like that. And besides that, the other issues were resolved everything was working fine? If so I can make the PR into galaxy dev

@dsavchenko
Copy link

weird that suddenly you have to force the folder creation

I don't really think this is weird. There is no such dir in the beginning, but you attempt to copy there in the build command.
I checked other tools, which have this dir after build, it is either already in the repo or explicitly created in the build command.

And besides that, the other issues were resolved everything was working fine?

Looks ok, the main issue was that it degraded somehow and was not working at all either on usegalaxy-eu or with fresh local instance. Now it works locally, so I hope it will also work elsewhere.

The only thing I notice is 404 for /plugins/visualizations/fits_image_viewer/CDS/P/DSS2/color/properties.txt, but it could be expected, I'm not sure

@francoismg
Copy link
Author

francoismg commented Nov 2, 2023

I don't really think this is weird. There is no such dir in the beginning, but you attempt to copy there in the build command. I checked other tools, which have this dir after build, it is either already in the repo or explicitly created in the build command.

Yes you're right I think maybe there was no need to force folder creation before because cp was copying the folder directly instead of just its content like it does now, I don't know. But it's weird that I got no error when testing it, maybe I just forgot to remove that specific folder

And besides that, the other issues were resolved everything was working fine?

Looks ok, the main issue was that it degraded somehow and was not working at all either on usegalaxy-eu or with fresh local instance. Now it works locally, so I hope it will also work elsewhere.

The only thing I notice is 404 for /plugins/visualizations/fits_image_viewer/CDS/P/DSS2/color/properties.txt, but it could be expected, I'm not sure

Yes I think the properties.txt 404 is expected, think it was there from the beginning.

So the mime type issue on galaxy.eu should be resolved by the use of the cdn

And the misplaced aladin files should be good too, can you just confirm that the files are in /dist/aladin-lite-galaxy in the main static folder /static/plugins/visualization/fits_image_viewer/static on your install please?

@francoismg
Copy link
Author

francoismg commented Nov 2, 2023

Here are contents of static/plugins/visualizations/fits_image_viewer

.
└── static
    ├── dist
    │   └── aladin-lite-galaxy
    │       ├── 049a5201c34949755ec5.module.wasm
    │       ├── 642.aladin.js
    │       ├── 7538bf853f9b14029f986310747a9aad.png
    │       ├── aladin.js
    │       ├── aladin.js.LICENSE.txt
    │       ├── ccdb93d24585bd08a4261722b95e9e3d.png
    │       └── package.json
    ├── logo.png
    ├── plugin_build_hash.txt
    ├── script.js
    ├── script.js.map
    └── style.css

ok thanks, everything should be ok then. I will open the pr to the galaxy dev branch.

I did some tests and the cp command creates the missing folder either when directly copying the folder or when copying folder content so maybe it behaves differently on macos I don't know but it's the only thing I can think of.

@volodymyrss
Copy link

Could you link the PR here, @francoismg ?

@francoismg
Copy link
Author

galaxyproject#17002

@francoismg
Copy link
Author

francoismg commented Nov 13, 2023

@dsavchenko I have updated the fix branch, could you try it on your machine to be sure that it's working for you please?

https://github.com/esg-epfl-apc/galaxy/tree/fix-visualization-plugin

@dsavchenko
Copy link

could you try it on your machine to be sure that it's working for you please?

I confirm it works

@francoismg
Copy link
Author

@bgruening we are still experiencing the same error with the plugin, do you know if the new release has already been pushed to usegalaxy.eu? If that's the case could you check if it's the same meme type error we had or something else? Thanks

@bgruening
Copy link
Member

We are still running 23.1 - have you commited it against this branch? Otherwise we can also backport it to here.

@dsavchenko
Copy link

We are still running 23.1 - have you commited it against this branch? Otherwise we can also backport it to here.

I checked today, the needed fix is not in 23.1. It would be great to backport it there. Does this require an action from our side?

@bgruening
Copy link
Member

Then it should be deployed, can you please check this repo here?

@volodymyrss
Copy link

Then it should be deployed, can you please check this repo here?

You mean the repo on which we are, this one? To make a PR to the release branch?

If you @bgruening confirm, probably best of @francoismg does a PR.

@francoismg
Copy link
Author

Then it should be deployed, can you please check this repo here?

You mean the repo on which we are, this one? To make a PR to the release branch?

If you @bgruening confirm, probably best of @francoismg does a PR.

Ok I wasn't sure what was meant by "check this repo here"

The fix will be available in this release https://github.com/galaxyproject/galaxy/tree/release_23.2/config/plugins/visualizations/fits_image_viewer apparently, but I can make a new PR to this repo (/use-galaxy-eu) if needed no problem

@volodymyrss
Copy link

Then it should be deployed, can you please check this repo here?

You mean the repo on which we are, this one? To make a PR to the release branch?
If you @bgruening confirm, probably best of @francoismg does a PR.

Ok I wasn't sure what was meant by "check this repo here"

I was not quite sure too, but that's my interpretation given the available information. @bgruening please confirm :)

The fix will be available in this release https://github.com/galaxyproject/galaxy/tree/release_23.2/config/plugins/visualizations/fits_image_viewer apparently, but I can make a new PR to this repo (/use-galaxy-eu) if needed no problem

This depends entirely on how soon 23.2 will be on usegalaxy.eu. Since @bgruening suggests to backport, I think we should not wait till 23.2, and backport now.
Unless @bgruening protests, please @francoismg make a PR to the 23.1-EU release branch, looks like people are doing that.

We need this feature, we advertised it and it's hard to promote our work when it's not there.

@bgruening
Copy link
Member

Ok, sorry that I'm not clear. This repo is here in which we are: https://github.com/usegalaxy-eu/galaxy

is the deployed version of usegalaxy.EU ... we try to be as close as possible to the release_23.1 branch of upstream, but might carry around a few patches from dev or a few new Vis etc from EU communities.

Hope that clarifies things. If it is in upstream release_23.1 then it should be also here in release_23.1_europe.

@bgruening
Copy link
Member

This depends entirely on how soon 23.2 will be on usegalaxy.eu. Since @bgruening suggests to backport, I think we should not wait till 23.2, and backport now.

2-3 weeks. We have a big workshop next week and then need to get people back from vacation :)

@bgruening
Copy link
Member

Unless @bgruening protests, please @francoismg make a PR to the 23.1-EU release branch, looks like people are doing that.

I'm not protesting ...

@francoismg
Copy link
Author

This depends entirely on how soon 23.2 will be on usegalaxy.eu. Since @bgruening suggests to backport, I think we should not wait till 23.2, and backport now. Unless @bgruening protests, please @francoismg make a PR to the 23.1-EU release branch, looks like people are doing that.

Ok no problem will do that

@volodymyrss
Copy link

Thanks @bgruening for clarification!
We could in principle wait 2-3 weeks, but the backport is hopefully not too complex, and I hope the deployment of the next 23.1 dev version is not a big trouble. @francoismg please mention the PR here for tracking.

Overall, we on our side need to learn how to engage and anticipate big instance development so that we can contribute effectively. Somehow this feature was out of commission for 4 month so far, it's harder to show-off and disseminate in our broader community without it :(
I am sure we will figure it out.

@bgruening
Copy link
Member

I'm perfectly fine to backport those changes as long as you help us :)

@bgruening
Copy link
Member

Overall, we on our side need to learn how to engage and anticipate big instance development so that we can contribute effectively.

The other important repo is here: https://github.com/usegalaxy-eu/infrastructure-playbook
And a bit of (internal) documentation about our instance is here: https://github.com/usegalaxy-eu/operations

Happy to give you more info and answer all questions.

@volodymyrss
Copy link

Overall, we on our side need to learn how to engage and anticipate big instance development so that we can contribute effectively.

The other important repo is here: https://github.com/usegalaxy-eu/infrastructure-playbook And a bit of (internal) documentation about our instance is here: https://github.com/usegalaxy-eu/operations

Happy to give you more info and answer all questions.

Thanks! Looks a good basis to get an idea about when and how to make the kind of contribution we make, making Galaxy more convenient to our community.
Maybe it can be even mentioned in some guideline or testament on our galaxy community development.

@bgruening
Copy link
Member

Sure, its all public and we are happy to enhance our documentation.

@francoismg
Copy link
Author

The PR for the release branch is available here #218

@bgruening
Copy link
Member

Ok, this is now deployed! In general, we deploy all changes in the European night. I have just run it now.

@francoismg
Copy link
Author

francoismg commented Jan 25, 2024

Ok, this is now deployed! In general, we deploy all changes in the European night. I have just run it now.

Thanks a lot, it seems to be working fine now.

Screenshot 2024-01-25 at 01 12 47

@volodymyrss
Copy link

volodymyrss commented Jan 25, 2024

Thanks, I confirm it works, although I needed to clear site data (browser cache). So we'd need to ask all users to do that.
I see that some assets (e.g. JS) include versions, but some other (including our plugin JS) do not.
What is the best way to reliably update JS assets in galaxy, to avoid asking all users for an action?

@bgruening
Copy link
Member

You mean the browser cache?

@volodymyrss
Copy link

You mean the browser cache?

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants