The Wayback Machine - https://web.archive.org/web/20210227232015/https://github.com/TextureGroup/Texture/pull/1948
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

Exposes a new option in ASImageDownloaderProtocol to retry image downloads #1948

Merged
merged 3 commits into from Feb 19, 2021

Conversation

@chggr
Copy link
Contributor

@chggr chggr commented Jan 22, 2021

At the moment the ASBasicImageDownloader does not automatically retry image downloads if the remote
host is unreachable. On the contrary the ASPINRemoteImageDownloader automatically retries. Retrying is
something that ultimately clients need to be able to control, for example to fail fast to an alternative image
rather than keep retrying for more than one minute while not displaying any image. This change exposes
a new option in the ASImageDownloaderProtocol to retry image downloads. It also uses this new option
in both ASNetworkImageNode and also ASMultiplexImageNode, setting it to YES to preserve the current
behaviour.

chggr added 2 commits Jan 22, 2021
…loads

At the moment the ASBasicImageDownloader does not automatically retry image downloads if the remote
host is unreachable. On the contrary the ASPINRemoteImageDownloader automatically retries. Retrying is
something that ultimately clients need to be able to control, for example to fail fast to an alternative image
rather than keep retrying for more than one minute while not displaying any image. This change exposes
a new option in the ASImageDownloaderProtocol to retry image downloads. It also uses this new option
in both ASNetworkImageNode and also ASMultiplexImageNode, setting it to YES to preserve the current
behaviour.
@chggr
Copy link
Contributor Author

@chggr chggr commented Jan 27, 2021

Hi team, just wanted to check whether you had any feedback on this PR. Thanks!

ScreenNode.m is implementing ASImageDownloaderProtocol and needs to
be fixed to reflect changes in the latter.
@CLAassistant
Copy link

@CLAassistant CLAassistant commented Jan 31, 2021

CLA assistant check
All committers have signed the CLA.

@chggr
Copy link
Contributor Author

@chggr chggr commented Feb 19, 2021

Hi team, just a friendly ping, who would be the best person to review and provide feedback / merge this PR?

Copy link
Member

@bolsinga bolsinga left a comment

The only nit I have is to back fill tests that the PINRemoteImageManagerDownloadsOptions are populated correctly. Thanks!

@bolsinga bolsinga merged commit 17d4d13 into TextureGroup:master Feb 19, 2021
11 checks passed
11 checks passed
Verify that default subspecs lint
Details
Build and run tests
Details
Verify that other subspecs lint
Details
Build Texture as a dynamic framework
Details
Build Texture as a static library
Details
Verify that Carthage works
Details
Build examples (examples-pt1)
Details
Build examples (examples-pt2)
Details
Build examples (examples-pt3)
Details
Build examples (examples-pt4)
Details
license/cla Contributor License Agreement is signed.
Details
@chggr chggr deleted the chggr:retryImageDownload branch Feb 20, 2021
rcancro added a commit to rcancro/Texture that referenced this pull request Feb 23, 2021
…loads (TextureGroup#1948)

* Exposes a new option in ASImageDownloaderProtocol to retry image downloads

At the moment the ASBasicImageDownloader does not automatically retry image downloads if the remote
host is unreachable. On the contrary the ASPINRemoteImageDownloader automatically retries. Retrying is
something that ultimately clients need to be able to control, for example to fail fast to an alternative image
rather than keep retrying for more than one minute while not displaying any image. This change exposes
a new option in the ASImageDownloaderProtocol to retry image downloads. It also uses this new option
in both ASNetworkImageNode and also ASMultiplexImageNode, setting it to YES to preserve the current
behaviour.

* Fixes a failing test in ASMultiplexImageNodeTests

* Fixes ScreenNode.m

ScreenNode.m is implementing ASImageDownloaderProtocol and needs to
be fixed to reflect changes in the latter.
rcancro added a commit to rcancro/Texture that referenced this pull request Feb 26, 2021
…loads (TextureGroup#1948)

* Exposes a new option in ASImageDownloaderProtocol to retry image downloads

At the moment the ASBasicImageDownloader does not automatically retry image downloads if the remote
host is unreachable. On the contrary the ASPINRemoteImageDownloader automatically retries. Retrying is
something that ultimately clients need to be able to control, for example to fail fast to an alternative image
rather than keep retrying for more than one minute while not displaying any image. This change exposes
a new option in the ASImageDownloaderProtocol to retry image downloads. It also uses this new option
in both ASNetworkImageNode and also ASMultiplexImageNode, setting it to YES to preserve the current
behaviour.

* Fixes a failing test in ASMultiplexImageNodeTests

* Fixes ScreenNode.m

ScreenNode.m is implementing ASImageDownloaderProtocol and needs to
be fixed to reflect changes in the latter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants