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

Remove the local file in test_download_dataset before download #5887 #5888

Conversation

joeldierkes
Copy link
Contributor

The local created file in test_download_dataset, which gets uploaded, is not
removed before the download. This results in a failing test, since it is cached
and thus does not need to be downloaded.

…o#5887

The local created file in `test_download_dataset`, which gets uploaded, is not
removed before the download. This results in a failing test, since it is cached
and thus does not need to be downloaded.
@joeldierkes joeldierkes linked an issue Sep 21, 2022 that may be closed by this pull request
Copy link
Contributor

@yuyiguo yuyiguo left a comment

Choose a reason for hiding this comment

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

Should we check if the file is there or not, before removing it? Because sometimes the test does pass like Martin restarted the test today.

@joeldierkes
Copy link
Contributor Author

The problem I have with that is that the file should not be deleted in the first place. Uploading a file does not delete it to my knowledge. I think the test fails because due to some internal behavior the file is recognized sometimes, and sometimes not (which is a very unsatisfying statement, I know).

I think it would be beneficial to know if the file gets deleted and thus can not be deleted anymore, since this should not happen in the first place.

@joeldierkes
Copy link
Contributor Author

@yuyiguo @bari12 does that make sense?

@yuyiguo
Copy link
Contributor

yuyiguo commented Sep 21, 2022

How about below?

try:
os.remove(file)
except:
pass

@joeldierkes
Copy link
Contributor Author

I think it would be beneficial to know if the file gets deleted and thus can not be deleted anymore, since this should not happen in the first place.

Okay, this sentence is probably a bit ambiguous.
I meant to say that we want to know if the file does not exist anymore, since we don't explicitly delete it. I think it would not be beneficial to silent the error.

@yuyiguo
Copy link
Contributor

yuyiguo commented Sep 21, 2022

I got you now. It is depending on what the test is for. The current fixing will give the failing test to someone whose PR may not relate to the failed issue at all. But it is ok if the test will eventually be fixed after we know why the file sometimes "disappeared".

@bari12 bari12 merged commit 4e2556f into rucio:master Sep 30, 2022
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.

Infrequent failing test_download_dataset test
3 participants