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

Download feed logos via guzzle to have better error handling #1533

Merged
merged 4 commits into from
Oct 18, 2021
Merged

Conversation

Grotax
Copy link
Member

@Grotax Grotax commented Oct 15, 2021

This PR attempts to replace php's simple copy function by guzzle.

  • fix long filenames causing errors
  • fix downloading files every time
  • fix general errors copy can't handle
  • silence errors to info, logos are not that important

@Grotax Grotax changed the title Use guzzle to download feed logos Download feed logos via guzzle to have better error handling Oct 16, 2021
@Grotax
Copy link
Member Author

Grotax commented Oct 16, 2021

Related: #1527, #1464, #1318, #633, #1510

@Grotax
Copy link
Member Author

Grotax commented Oct 16, 2021

Runs fine on my dev server, will deploy this on my prod server.

@Grotax
Copy link
Member Author

Grotax commented Oct 16, 2021

If someone wants to test, build and officially signed by me (no new version number)
news.tar.gz

@codecov-commenter
Copy link

codecov-commenter commented Oct 17, 2021

Codecov Report

Merging #1533 (3ce07da) into master (c0998b2) will decrease coverage by 0.63%.
The diff coverage is 8.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1533      +/-   ##
============================================
- Coverage     92.00%   91.37%   -0.64%     
- Complexity      766      768       +2     
============================================
  Files            65       65              
  Lines          2803     2816      +13     
============================================
- Hits           2579     2573       -6     
- Misses          224      243      +19     
Impacted Files Coverage Δ
lib/Fetcher/FeedFetcher.php 78.44% <8.00%> (-10.23%) ⬇️
lib/Service/FeedServiceV2.php 100.00% <0.00%> (ø)
lib/Command/Debug/ItemList.php 100.00% <0.00%> (ø)
lib/Command/Debug/FeedItemList.php 100.00% <0.00%> (ø)
lib/Command/Debug/FolderItemList.php 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c0998b2...3ce07da. Read the comment docs.

lib/Fetcher/FeedFetcher.php Outdated Show resolved Hide resolved
lib/Fetcher/FeedFetcher.php Outdated Show resolved Hide resolved
Grotax and others added 2 commits October 17, 2021 15:15
Signed-off-by: Benjamin Brahmer <[email protected]>

Co-authored-by: Sean Molenaar <[email protected]>
Signed-off-by: Benjamin Brahmer <[email protected]>

Co-authored-by: Sean Molenaar <[email protected]>
@Grotax
Copy link
Member Author

Grotax commented Oct 17, 2021

New version including all the changes.

news.tar.gz

@Grotax Grotax merged commit 137af71 into master Oct 18, 2021
@delete-merged-branch delete-merged-branch bot deleted the favicon branch October 18, 2021 15:57
@Grotax
Copy link
Member Author

Grotax commented Oct 18, 2021

I plan to extend this solution to also support proxies and make use of the general config, but incremental working changes are better than the big bang that doesn't work.

Grotax added a commit that referenced this pull request Oct 18, 2021
Changed
- Add changelog and DCO notice to CONTRIBUTING.md (#1521)
- Download feed logos via guzzle to have better error handling (#1533)

Signed-off-by: Benjamin Brahmer <[email protected]>
@Grotax Grotax mentioned this pull request Oct 18, 2021
Grotax added a commit that referenced this pull request Oct 20, 2021
Changed
- Add changelog and DCO notice to CONTRIBUTING.md (#1521)
- Download feed logos via guzzle to have better error handling (#1533)

Signed-off-by: Benjamin Brahmer <[email protected]>
Neo11 pushed a commit to Neo11/news that referenced this pull request May 28, 2022
Changed
- Add changelog and DCO notice to CONTRIBUTING.md (nextcloud#1521)
- Download feed logos via guzzle to have better error handling (nextcloud#1533)

Signed-off-by: Benjamin Brahmer <[email protected]>
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