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

assets: warn the user when a hash is not provided #4539

Merged
merged 1 commit into from Apr 21, 2021

Conversation

wainersm
Copy link
Contributor

If the Asset.fetch() caller does not provide a hash for the asset then
it is not possible to check its integrity. Problems may arise from using
corrupted files. So let's log a message warning the user about it.

Signed-off-by: Wainer dos Santos Moschetta wainersm@redhat.com

@wainersm wainersm added the customer:QEMU Requirements/issues raised by the QEMU project label Apr 19, 2021
@wainersm
Copy link
Contributor Author

This improvement raised from a discussion on QEMU: https://www.mail-archive.com/qemu-devel@nongnu.org/msg800013.html

@wainersm wainersm requested a review from clebergnu April 19, 2021 22:25
avocado/utils/asset.py Outdated Show resolved Hide resolved
Copy link
Member

@beraldoleal beraldoleal left a comment

Choose a reason for hiding this comment

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

Hi @wainersm thank you for this. This is very welcome. Just one small suggestion and we are good to go.

IMO, the entire asset think must be redesigned, and we are planning to do this soon. I have a blueprint to write and @clebergnu is doing another one and hopefully, they will be in alignment.

avocado/utils/asset.py Outdated Show resolved Hide resolved
If the Asset.fetch() caller does not provide a hash for the asset then
it is not possible to check its integrity. Problems may arise from using
corrupted files. So let's log a message warning the user about it.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
@wainersm
Copy link
Contributor Author

@beraldoleal @richtja

Just updated the commit with:

  • warning() instead of info()
  • on the message, "check" instead of "ensure"

@codeclimate
Copy link

codeclimate bot commented Apr 20, 2021

Code Climate has analyzed commit 2f2ffa4 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 68.3% (0.0% change).

View more on Code Climate.

@wainersm
Copy link
Contributor Author

@richtja @beraldoleal the failed tests on travis-ci seems not related with my changes. Is this job known to fail intermittently?

@richtja
Copy link
Contributor

richtja commented Apr 21, 2021

@wainersm Yes this is known Travis issue. Thanks for the changes.

@richtja richtja merged commit 2cc4646 into avocado-framework:master Apr 21, 2021
@richtja richtja added this to Review Requested in Avocado Kanban via automation Apr 21, 2021
@richtja richtja added this to the #88 (The Serpent) milestone Apr 21, 2021
@richtja richtja moved this from Review Requested to Done #88 (The Serpent) in Avocado Kanban Apr 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer:QEMU Requirements/issues raised by the QEMU project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants