Closed ckuethe closed 7 months ago
Thanks for this Chris!
I'm a bit torn on one part though: I agree it seems a little wrong to have a different function signature of startWebServer
, depending if its meant for localhost vs web-serving; however, it also seems about as wrong to instead ignore a value of the http_address
argument when BUILD_FOR_WEB_DEPLOYMENT
is not defined. The reason I originally went for the different signatures, was to extra make sure that for any build that might be made external to your computer (there are a few different use cases for this), you had to be explicit over how things were initialized (since files outside of the InterSpec deployment could potentially be accessed), and this was really what you wanted the code to do, relative to localhost builds.
Also, the BUILD_FOR_WEB_DEPLOYMENT=ON
option is not something that I have used or tested for a couple years now, so please use caution.
I have a busy next few days, so I'll think some more on this, and come back to this.
again, thank you!
No worries, I didn't think this diff would land right away. It just seems easier to reason about the purpose and future of BUILD_FOR_WEB_DEPLOYMENT
if there is a diff to discuss.
For the local deployment case, I'll rework the logic to not ignore the http-address
parameter and instead loudly enforce a requirement that it somehow resolve to localhost (ie. leave it unspecified to use the localhost default, or explicitly say "localhost" or "127.0.0.1"). In other words, if you provide a not-localhost address to a localhost build, it will fail to start and tell you why. I saw the docker target which is what got me started down this rabbit hole in the first place.
Data sharing/isolation/security is on my mind. Single-user-multi-display is my primary use case for web deployment, but I know some other people who also think gamma ray spectroscopy is fun. For now, they can install interspec on their devices and can all work out of a shared drive. It might be useful if we could have a central interspec instance backed by some shared storage. I assume there are still other interspec users who turned pale and shouted "oh heck no!" at the thought of exposing their spectra and analysis on the internet. In a way this isn't too different from Jupyter. I run jupyter on my laptop for quick prototypes, and on my workstation where I have more resources. I could put a reverse proxy in front of that so I could hit it remotely. And I've seen jupyter deployed in business setting using various auth and isolation models from a single host with a single shared password, all the way up to dockerized with a container per user and auth via SSO.
Again, this is for discussion and there's no need to merge this urgently. I'd rather take the time to figure out the right way than to quickly ship bugs.
Thanks for the feedback, I'll have an updated PR for you shortly.
Whatever skin it's wearing InterSpec is a web app. It smells wrong to me to change the function signature of start_server and friends depending on whether BUILD_FOR_WEB_DEPLOYMENT or BUILD_AS_LOCAL_SERVER are defined, when all the same data files are being used and all javascript and HTML is being served no matter what display engine is being used.
This diff does a few things:
--http-address
optional, and uses the localhost default if not otherwise specifiedTested with