implement custom implementation of obtain_file method in QuantumESPRESSO easyblock#618
implement custom implementation of obtain_file method in QuantumESPRESSO easyblock#618boegel wants to merge 1 commit intoeasybuilders:developfrom
Conversation
|
Refer to this link for build results (access rights to CI server needed): This pull request is now ready for review/testing. Please try and find someone who can tackle this; contact @boegel if you're not sure what to do. |
|
@wpoely86, @JensTimmerman: thoughts on this? I'm not sure how happy I am with this, but it does fix the issue at hand. |
|
so what is the problem here? It returns a html page instead of a 404 ? |
|
@wpoely86: yes, for example: http://www.qe-forge.org/gf/download/frsrelease/185/753/GWW-5.1.2.tar.gz (vs http://www.qe-forge.org/gf/download/frsrelease/185/754/GWW-5.1.2.tar.gz, which is the valid URL) |
|
Is this not worth adding the framework then? |
|
@wpoely86: maybe, I'm not sure, this is the first time we run into an issue like this, afaik |
There was a problem hiding this comment.
you're ignoring every EasyBuildError, not just download errors?
|
This should probably be fixed somewhere in the framework, instead of just this EasyBlock? |
There was a problem hiding this comment.
only if it is set, which is still a minority of the easyconfigs.
There was a problem hiding this comment.
sure, but it's a lot faster and easier to set the md5sum for the sources here then to try to fix it this way?
There was a problem hiding this comment.
If there's an MD5sum available, it will detect this issue, yes, but simply complain that things don't match.
Checking the MD5sum if you know that the download is faulty is pretty useless.
There was a problem hiding this comment.
the sum only get's checked after the download stage, not after each individual download?
|
Why is this special workaround needed? Can't the md5sum pick this failure to download up? The download problem is bigger then just for this package, we see this all over EasyBuild, links that go missing etc, so should be tackled on a bigger level, not on an individual basis? |
|
I agree with @JensTimmerman here. Should be handled in the framework. Does a simple check on the return mimetype not suffice to check whether we get html or something else? |
|
The problem is you then have to specify in the easyconfig what mime type you are expecting, the extension doesn't really mean anything anyway, but of course, it could be used as an indicator, I have some code in a branch that checks magic numbers, so we could spit out warnings if magic numbers don't match up with the extension... We could do that, but since we have the checksum infrastructure in place already I suggest we use that? Anyway, not something that should be fixed here? |
|
Why not do both? Checksums are the best option but it's nice to have a fallback. Do we actually need the magic numbers stuff? Do we have any cases where the content-type is |
|
The problem here is that without having this in place, the download for QuantumESPRESSO can never work. The non-generic URLs used in easybuilders/easybuild-easyconfigs#1644 don't yield a 404 if a download is attempted that is not available at that URL; the generic URL does, but not everything can be downloaded that way. So, without this workaround, auto-download for QuantumESPRESSO is simply broken, and there's no way around it by reordering the I agree that this fix can be implemented more generally, and maybe it should (and be disabled by default). But we've never run into an issue like this, i.e. source URLs that do not properly yield a 404 when attempting to download something that isn't there. So this problem is specific to QuantumESPRESSO, for now. |
|
My point about the md5sum is that this is the way to figure out if a download is correct or not, So can the general framework code be changed to check the md5sum after a download, and if that's wrong, move on to the next source url? |
|
What if checksums are not available? (as they are now, in QuantumESPRESSO easyconfigs)? |
|
then you have to add the checksums to the easyconfig, or fix it in the framework, instead of trying to patch it in in the easyblock? |
|
With #3257, we're moving to a whole new easyblock for QE, so closing this |
5 participants
No description provided.