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

Implement own screenshot generator using node-webshot #6

Merged
merged 9 commits into from
Mar 9, 2016

Conversation

outcoldman
Copy link
Member

No description provided.

@outcoldman
Copy link
Member Author

@artemgrygor working prototype!

After giving some thoughts:

  • I would not care about the authentication. We are generating screenshots in specific size, I doubt that somebody will need them in the same size. So I would not care about it right now.
  • For the caching - I don't think we should really care about it. We can use CloudFront or CloudFlare and their caching capabilities for now.

@artemgrygor
Copy link
Contributor

@outcoldman awesome, you're super fast :-)
Good point about size! I see there is a docker file, so I assume we will use DO?

artemgrygor added a commit that referenced this pull request Mar 9, 2016
Implement own screenshot generator using node-webshot
@artemgrygor artemgrygor merged commit 17766cc into dev Mar 9, 2016
@jamiewilson
Copy link
Member

@outcoldman Pulled these changes and I'm getting errors on all screenshots here. it says that it's blocked by the Cross-Origin Resource Sharing policy: No 'Access-Control-Allow-Origin' header.

https://gist.github.com/jamiewilson/da7436ff4934c4d8559b

@outcoldman
Copy link
Member Author

@jamiewilson we set Access-Control-Allow-Origin https://github.com/deweyapp/dewey-server/blob/master/app.js#L8, also CloudFlare should pass them https://support.cloudflare.com/hc/en-us/articles/200308847-Does-CloudFlare-support-Cross-origin-resource-sharing-CORS-

@artemgrygor could you verify as well?

@jamiewilson could you give it another try? possible that you tried when I was changing something on the server.

@jamiewilson
Copy link
Member

@outcoldman Okay, cross origin working now. Two additional things:

  1. If possible, it might be good to timeout the screen capture for 1 second or so in order to allow for any loading animations to take place. For example, my website.
  2. It looks like links such as domain.com/internal-page are failing. Are we limited to only capturing screenshots at the root level of sites?

@outcoldman
Copy link
Member Author

@jamiewilson yeah domain.com/internal-page probably does not work, it should be a http://domain.com/internal-page

@outcoldman
Copy link
Member Author

@jamiewilson and about the 1 second - because we are limiting number of concurrent requests - everytime we wait 1 second before making screenshots - we delay screenshot generation for ALL other requests/users and also for this request as well. There are very limited number of websites which do that. I just don't think that we can afford it at current moment.

@outcoldman
Copy link
Member Author

@jamiewilson I will add renderDelay=200 (200 ms), will that work?

@jamiewilson
Copy link
Member

That makes sense about the delay. I think you're right. We should probably lean towards producing them quickly for now and then if it makes sense later we can add the renderDelay if needed.

Also, are you seeing any problems with rendering images for https addresses?

@outcoldman
Copy link
Member Author

@jamiewilson have not, it seems like because we are limited in the memory (512Mb for this droplet) we are getting a lot of 400 results. I am still trying to find the best settings, how many concurrent requests we should do and so on. Maybe will try to switch to 1Gb to try.

@artemgrygor
Copy link
Contributor

@outcoldman @jamiewilson sorry for the delay was busy during the day. I can't check it on mac. Can't actually do npm install due to this error on Mac (nodejs/node-gyp#363) and their solution doesn't work. Can check only tomorrow on windows machine.

@artemgrygor
Copy link
Contributor

@outcoldman @jamiewilson I confirm, it works on windows (just had to install ImageMagic exe with headers, probably need to do the same on mac).
Btw, yes, I got cross-origin firstly. Probably just the cache.

@outcoldman
Copy link
Member Author

@artemgrygor @jamiewilson 👍

It is up and running now. I published new version to Chrome Store.

@artemgrygor please keep your server running for a bit, soon we will be able to shut it down.

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

Successfully merging this pull request may close these issues.

3 participants