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

fix: issue1460 intermediate files existing (or not) #1541

Merged
merged 7 commits into from Apr 2, 2022

Conversation

vsoch
Copy link
Contributor

@vsoch vsoch commented Mar 30, 2022

Description

Just testing different ideas for #1460 - feel free to ignore!

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).

@github-actions
Copy link
Contributor

Please format your code with black: black snakemake tests/*.py.

@vsoch vsoch changed the title Fix vsoch/issue1460 fix: issue1460 intermediate files existing (or not) Mar 30, 2022
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
the issue is that the blob object held by a GS remote object can go sort
of stale, returning a False for blob.exists() when it clearly exists! To fix
we need to do an additional self.update_blob() and then returning the exists
check again. I am not sure if this can be made more efficient by only checking
under certain conditions, but since it seems likely we cannot perfectly know when
the blob has gone stale the sure way is to always update.

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch
Copy link
Contributor Author

vsoch commented Apr 2, 2022

okay doing a little testing - I've gotten down to the io.py function to wait for files, and looking at the files and attributes, according to this, one of them does not exist:

image

indeed it does exist!

image

It comes down to checking if a remote object exists, e.g., self.blob.exists() erroneously returns False:

Type 'copyright', 'credits' or 'license' for more information
IPython 7.23.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: self.blob
Out[1]: <Blob: snakemake-testing-d924es45, test_github_issue1460/preblob.txt, 1648855265000563>

In [2]: self.blob.exists()
Out[2]: False

but there it is!

image

self.blob.reload() does not work

self.blob.reload()
...
NotFound: 404 GET https://storage.googleapis.com/storage/v1/b/snakemake-testing-d924es45/o/test_github_issue1460%2Fpreblob.txt?generation=1648855265000563&projection=noAcl&prettyPrint=false: No such object: snakemake-testing-d924es45/test_github_issue1460/preblob.txt

But indeed if we "ask again" for the key, we see it exists:

In [11]: self.bucket.get_blob(self.key)
Out[11]: <Blob: snakemake-testing-d924es45, test_github_issue1460/preblob.txt, 1648856699412397>

In [12]: blob = self.bucket.get_blob(self.key)

In [13]: blob.exists()
Out[13]: True

so that's the change I did for the PR here - looks like the reproducing test of the bug passes, so we fixed it! There might be some ways to further optimize (e.g., only do the update_blob under some conditions) but someone else can look into that if desired.

Thanks for letting me play on GCP this evening @johanneskoester it was SO fun I missed it a lot <3

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@sonarcloud
Copy link

sonarcloud bot commented Apr 2, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@johanneskoester
Copy link
Contributor

Awesome work @vsoch, thanks a lot!

@johanneskoester johanneskoester merged commit 1b3ede1 into main Apr 2, 2022
@johanneskoester johanneskoester deleted the fix-vsoch/issue1460 branch April 2, 2022 06:36
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.

None yet

2 participants