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.

Sign up for GitHub

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

Jump to bottom

create: check HTML response is 200 when fetching logo #1369

Closed
ggabernet opened this issue Dec 15, 2021 · 10 comments · Fixed by #1450
Closed

create: check HTML response is 200 when fetching logo #1369

ggabernet opened this issue Dec 15, 2021 · 10 comments · Fixed by #1450
Labels
bug Something isn't working command line tools Anything to do with the cli interfaces

Comments

@ggabernet
Copy link
Member

ggabernet commented Dec 15, 2021 •
edited by ewels
Loading

Description of the bug

After an automated sync of the nf-core tools v2.2, the dark logo for Epitopeprediction was not created and instead this was inside that file:

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>507 Insufficient Storage</title>
</head><body>
<h1>Insufficient Storage</h1>
<p>The method could not be performed on the resource
because the server is unable to store the
representation needed to successfully complete the
request.  There is insufficient free space left in
your storage allocation.</p>
<p>Additionally, a 507 Insufficient Storage
error was encountered while trying to use an ErrorDocument to handle the request.</p>
</body></html>

I obtained the same results when running nf-core sync locally.

This also causes an nf-core lint error as the file "is not equal to the template"

Command used and terminal output

nf-core sync

System information

I'm not sure what are the versions installed in the server.

Locally I was using NXF_VER = 21.10.5 and nf-core tools v2.2 on macOS
@ggabernet ggabernet added the bug Something isn't working label Dec 15, 2021
@mashehu
Copy link
Contributor

mashehu commented Dec 15, 2021

Okay, so you have it in the template, but it is not copied over correctly? Or why is the linting also complaining?

@ewels
Copy link
Member

ewels commented Dec 16, 2021

So the nf-core create command that is called by sync will be downloading that HTML instead of the dark logo from the nf-co.re website. I guess the code doesn't check whether we get a good response, so just saves whatever it gets to the file. Then linting fails because the file looks wrong..

I guess that this happens when the website is hit by all pipelines at once during the sync to generate a lot of images. It will have been especially severe here because we changed the filenames and added new logo, so instead of using the cached logo results, it will have needed to generate 100+ logos all at one time.

We could add some code to the create command to check a successful response, but as they should all be cached for future syncs I'm inclined to forget about it and it shouldn't happen again. If we change the logos again we should try to generate all of the logos manually prior to sync (I think I've done this before for the same reason actually).

@ewels ewels changed the title Error in creating logo with automated sync create: check HTML response is 200 when fetching logo Dec 16, 2021
@mashehu
Copy link
Contributor

mashehu commented Dec 16, 2021

Okay, sounds reasonable. How should we handle if the response is != 200? wait and try again?
In order to fix the sync related issue, should we add the logo creation as an explicit step?

@mashehu
Copy link
Contributor

mashehu commented Dec 16, 2021

Actually, I checked for cageseq and the image was correct in the assets directory (for the email template), but not in the docs. So, it doesn't look like a image creation / caching issue, more that it can't get everything delivered?

@ggabernet
Copy link
Member Author

The issue was only for the dark logo, which I think was only in the bin folder

@ewels
Copy link
Member

ewels commented Dec 18, 2021

I think that the docs and assets images are different resolutions? So will be treated as totally separate -> still a generation error (needs cache).

I think it's fine to just wait a few seconds and retry if we get != 200 for the fetch, maybe with max 5 retries or something? Hopefully that should get most through. I feel like we did something similar already for creating PRs which failed when we had a surge..?

@ewels ewels added the command line tools Anything to do with the cli interfaces label Dec 18, 2021
@jfy133
Copy link
Member

jfy133 commented Dec 22, 2021 •
edited by ewels
Loading

FWIW I got a separate error in the eager auto-sync

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<HTML><HEAD>
<TITLE> 508 Resource Limit Is Reached</TITLE>
</HEAD><BODY>
<H1>Resource Limit Is Reached</H1>
The website is temporarily unable to service your request as it exceeded resource limit.
Please try again later.
</BODY></HTML>

This was both in light and dark

@jfy133
Copy link
Member

jfy133 commented Dec 22, 2021

Also if someone comes searching for how to fix it nf-core lint --fix_files_unchanged recreates the images correctly 👍

@ggabernet
Copy link
Member Author

ah great to know @jfy133 🎉

ewels added a commit to ewels/nf-core-tools that referenced this issue Mar 15, 2022
@ewels
Updated nf-core create to tolerate failures and retry when fetching…
733e2e2 
… pipeline logos from the website

Fixes nf-core#1369
@ewels ewels mentioned this issue Mar 15, 2022
Merged
4 tasks
@ewels ewels linked a pull request Mar 15, 2022 that will close this issue
Updated nf-core create to tolerate failures and retry when fetching pipeline logos from the website #1450
Merged
4 tasks
@ewels ewels closed this as completed Mar 15, 2022
@ewels
Copy link
Member

ewels commented Mar 15, 2022

ok, now retries with varying wait times and even checks that the downloaded file looks like a PNG file 😎

Hopefully should resolve this issue..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working command line tools Anything to do with the cli interfaces
Projects
None yet
Milestone
No milestone
4 participants
@ewels @ggabernet @mashehu @jfy133