search

mattheworiordan / capybara-screenshot

Automatically save screen shots when a Capybara scenario fails
MIT License
1.02k stars 168 forks source link

Report S3 URL instead of local file path when S3 is used. #221

Closed rzaharenkov closed 6 years ago

AKoetsier commented 7 years ago

Would also be nice if you could generate signed urls to view the screenshots, that way you don't have to have an AWS account or set the images to public.

rzaharenkov commented 7 years ago

@AKoetsier, I've created a separate bucket for screenshots and made it public. Do you see any risks or problems with it?

AKoetsier commented 6 years ago

Well it is bad practice to open up your bucket to the world if you don't want everyone to see it. I would like to limit access since we only want to allow people who are able to login to drone to see the images. You can do that with signed urls.

AKoetsier commented 6 years ago

I have created a small patch that allows me to do so, I will try to push it this week.

ypresto commented 6 years ago

I have just setup s3 upload and found this issue. Could you push the patch? Thanks!

iggant commented 6 years ago

can you please concider to merge this, we're using bitbucket pipelines so local path is not very usefull, and really there is not much security with the public available screenshots

mattheworiordan commented 6 years ago

@rzaharenkov apologies I did not reply. I missed this somehow. Thank you for the contribution.

However, it's not passing CI is green. Would you mind rebasing to see if it now passes CI, and if does, I promise to quickly get this merged and get a release out.

@iggant thanks for the nudge.

leshik commented 6 years ago

@rzaharenkov would be nice to have base URL configurable... so that one could use any compatible object storage, such as minio. I'm using minio gateway to GCS, for example, so even the gateway URLs won't work. With the current implementation there also might be problems when using force_path_style: true option.

mattheworiordan commented 6 years ago

This is now resolved in https://github.com/mattheworiordan/capybara-screenshot/pull/239