Skip to content
This repository has been archived by the owner on Dec 11, 2020. It is now read-only.

Update Image.php #963

Merged
merged 3 commits into from
Sep 7, 2016
Merged

Update Image.php #963

merged 3 commits into from
Sep 7, 2016

Conversation

gulaandrij
Copy link
Contributor

fix broken image if service unavailable

@gulaandrij
Copy link
Contributor Author

any feedback on this?

@fzaninotto fzaninotto merged commit 3e55d05 into fzaninotto:master Sep 7, 2016
@fzaninotto
Copy link
Owner

Thanks!

Copy link

@ethanclevenger91 ethanclevenger91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silently deleting the image seems like unexpected behavior. I added a couple other things that might be updated if this strategy sticks. Thoughts?

$success = curl_exec($ch) || curl_getinfo($ch, CURLINFO_HTTP_CODE) === 200;

if ($success){
fclose($fp);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this happen regardless of success?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true

curl_close($ch);
fclose($fp);
} elseif (ini_get('allow_url_fopen')) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the same checks go here in case curl isn't enabled?

@gulaandrij
Copy link
Contributor Author

gulaandrij commented Sep 21, 2016

Silently deleting the image seems like unexpected behavior.

if lorempixel is not available then the image is saved as spoiled, 0 kb, so there is no need to store it

@ethanclevenger91
Copy link

Yeah, I get that much. But if I expect all my images to be generated, some are inexplicably missing, and I'm not versed in the intricacies of this issue, what do I do?

@gulaandrij
Copy link
Contributor Author

gulaandrij commented Sep 21, 2016

in my opinion its a good catch to add additional check in your code

if (!$faker->image()) {

} 

@ethanclevenger91
Copy link

But once again, if I'm a user who has no idea this is an issue, how would I know to do that?

I certainly don't do anything like:

if (!$faker->text()) {

}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants