NYPL-Simplified / webpub-viewer

8 stars 4 forks source link

SFR-599 deploy webpub viewer to elastic beanstalk #145

Closed hokei closed 4 years ago

hokei commented 4 years ago

Added deployment configurations for Elastic Beanstalk

Moved some devDependencies to dependencies so that the app will install in production See the deployed app here: http://researchnow-webpub-dev.us-east-1.elasticbeanstalk.com:4444/

EdwinGuzman commented 4 years ago

I think the original use of this was to be included into a project and not necessarily be the project itself. So you'd have a compiled version of it imported in a different app. If that can work, then you don't need to move those dev dependencies into the dependencies, which is preferable. I'm not sure what the architecture of RNow is but this won't be included into the main app?

EdwinGuzman commented 4 years ago

I'm going to check later how it was imported into the opds-web-client and get back to you.

hokei commented 4 years ago

Hmm ... I had been thinking of it as a standalone app this whole time, but a lot of things make a lot more sense if it isn't! The immediate plan was to replace the epub.js reader with this, one for one, as a temporary solution so that we can give UX an idea of what the app might look like with the new reader. If it's not a big lift to incorporate it the proper way, though, I'd rather do that!

EdwinGuzman commented 4 years ago

I'm not sure which is better and I don't know how the version of this with the most recent updates works. I believe for the Subway Library project we had a static version of it and that's similar to what I had in mind: a compiled or static version of the reader that can do what you want it to. That way, you don't need to download devdependencies into the main project. But, if this is meant to be standalone then it should be fine.

What does worry me is the beanstalk configs which are specific to RNow. Someone else using this project may have no need for those files (unless if they want to use it to scaffold their own project on EB).

hokei commented 4 years ago

If I understand what's going on, I don't think a static version of the reader is going to work here, because it's running zipped epubs through r2-streamer-js to be able to open and read them. I could look into bundling all the dependencies together, but I imagine it might get ... complicated? I don't know if this fact blocks us from incorporating this into RNow right now.

This isn't meant to be standalone long-term, it's just the fastest way to get RNow to be using this reader. I'm looking at opds-web-client now and seeing if that approach might work again, though I'm sure you took out webpub-viewer for a good reason and I'm not sure whatever was causing problems has been fixed/I can fix it

I totally see what you mean with the beanstalk configs. I'm not sure what we can do about them, even though I agree with you that this repo isn't really the place for them. I'm also a bit wary of things that are intended to be temporary - even though we go in with full intentions to change something, it can stick around for a while.

I think the way to move forward here is to look into whether this can be incorporated into RNow right away. If it can't, would it work as a temporary solution to push these configs onto another branch, and put something on RNow's roadmap to look into incorporating it as soon as we can?