-
Notifications
You must be signed in to change notification settings - Fork 7
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
Use a relative base URL to allow hosting of app in OMERO.web #59
base: dev
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for mdv-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
This simply saves you having to enter the dir when the page is refreshed, e.g. during development
f8a2a90
to
9ae7714
Compare
9ae7714
to
9c83bef
Compare
6a5b1cb
to
ff30874
Compare
@xinaesthete I realise that this branch (which I'm currently using in OMERO-MDV) was getting quite a long way behind your branch (and had merge conflicts) so I tried to resolve those conflicts and tested the resulting branch build in OMERO-MDV. It looks like there's a few things that have changed so this isn't currently working with OMERO-MDV. So I'm going to hold off on the rebase just now. Maybe we can discuss the best way to handle this process at some point, since it would be great to be able to keep up to date with the latest MDV features etc. Thx. |
Hi Will, Yes - I've been procrastinating about sorting out some aspects of my build, partly to do with how So, I'll review now, do whatever merging of main is needed into my branch, and may make some equivalent changes to yours that will hopefully work better for all of us... Best, |
Pretty sure there are no issues with Netlify & the relative base. I haven't been using or checking that very frequently, so it's always nice to realise it hasn't broken. I'll review your other changes and comments now. |
Thanks - I think that only the first commit e8dae46 can be cleanly merged into your branch. The 2nd commit 676ca1e I don't really need (and it conflicts with your branch). It was just for convenience when running MDV from the vite dev server, to avoid you having to enter a source The other commits I am using, to save views back to OMERO, including a CSRF token (I inject that into the page when I serve it) - see https://github.com/will-moore/omero-mdv/blob/709c5841dc2cc161d71d5c82ba7398cc927f78e5/omero_mdv/views.py#L366 I see that you have added a "save state" listener to POST the state, but it looks a bit different from the one I added (I think I was copying from the main branch or somewhere?), so I haven't yet checked if the format of the data is the same for both. The CSRF functionality should just do nothing if there's no CSRF token injected into the page. Would you be happy to merge that logic into MDV, with it being kinda specific to OMERO-MDV? Or is there a nicer way to achieve the same thing? Is the MDV app expecting the same content for |
I'll try again to resolve the merge conflicts and leave just the changes that I need... |
Yes... I started making a rebase of your branch locally and resolved that, no big deal I don't think, but also I think it's probably better for you to carry on with yours and then I can merge that.
I haven't given this much thought, I think it'd be fine to merge a version with the CSRF token.
As far as I know there is no difference other than the extension for |
The rebased app seems to be mostly working in OMERO-MDV. The datasources.js has sections like:
and the page is hosted at http://localhost:4080/omero_mdv/
Do I need to provide a different datasources.js config or something else? |
Another issue I'm finding is with "Save" of views back to OMERO. MDV/src/modules/static_index.ts Line 53 in e2589c1
Maybe I should try to use a different 'dir' URL so that staicFolder is False. |
So I resolved the merge conflicts which leaves nothing to change! Now I'm adding more changes to this branch that I need to get the updated MDV app to work with OMERO-MDV again (this will include CSRF changes), and at the same time I'm updating OMERO-MDV at will-moore/omero-mdv#9 The first of these changes is to use |
The next issue I'm currently stuck on is that the MDV app is sending a POST requests to
so it looks like it's expecting binary data for multiple columns, but I'm not sure how to combine them? |
Hi Will, Hopefully much of that will be fairly straightforward (famous last words), I'll review your comments in more detail soon. Best wishes |
I just had a look at this thread want to add some comments:- The serviceworker.js is just for static pages - it adds the headers to allow webworkers. Its just for static servers e.g. githubio (I also use it for a system we have in the WIMM , where you can put files in certain directory and they will be publicly served). It is not intended to be used with a 'proper backend' where you have control over which response headers that can be added I wanted to keep the backend and frontend as decoupled as possible, the dektop.config.js will build code that interacts with either the lightweight local server or static files and is bundled into the lightweight python server and also included when you create a static page. Again this code is not intended for a 'functional' server The prod.config.js (npm run build) builds a js file that exposes the ChartManager and a few generic methods (in the mdv 'namespace') for use with a dataloader. I imagine that people's use cases will be different so have left it up to the individual to handle loading and saving data. In the backend that I am developing at the moment, I have the the following code (no csrf tokens yet), which is similar to your code:-
The backend just does user authentication and then calls methods on an MDVProject e,g, to get data |
I've been making some more changes to how the vite config works - so now the entry is not an html file, and this may somewhat change how @will-moore uses it... although that said, my latest changes will also break my netlify build. So there is a risk that previously working scripts may break temporarily - LMK if there's any problems. Once the dust settles, it may make sense for OMERO-MDV to slightly change how it incorporates new versions - hopefully will be an improvement (maybe marginal). We should also figure out the best ways for you to configure data-loaders etc there... |
@will-moore I'm trying to do a bit of housekeeping and of course some things have changed a lot since you made this PR, but I'm making a bit of a note-to-self that you had relevant changes to do with Regards, |
d536533
to
5920117
Compare
@xinaesthete This is the tweak I needed to your branch to make it work in omero-mdv, since the app is hosted at
server/omero-mdv/
and all the static assets need to be served at e.g.server/omero-mdv/mdv.js
rather thanserver/mdv.js
.Hopefully this change shouldn't affect the way you're deploying to netlify etc?
If this change is deployed to e.g. netlify then I could try loading the app on the fly from there, as a possible alternative to serving the assets from omero-web itself.
UPDATE: in this branch, I have subsequently tried to merge in the
pjt-dev
branch which contains various changes and I haven't yet managed to get this to work with OMERO-mdv.So, I have checked-out a new branch at https://github.com/will-moore/MDV/tree/omero-mdv-build to keep track of the commit that I'm successfully using in omero-mdv.
Cheers,
Will