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

GridFS.get raises NoFile if file is not found #163

Merged
merged 5 commits into from
May 2, 2016
Merged

GridFS.get raises NoFile if file is not found #163

merged 5 commits into from
May 2, 2016

Conversation

touilleMan
Copy link
Contributor

As talked in #160

I've also corrected missing yield in db.fs.files.remove({}) which lead to concurrency errors

@touilleMan
Copy link
Contributor Author

CI failure (Failure: pymongo.errors.AutoReconnect: TxMongo lost connection to MongoDB.)
related to #164 : index creation is not over when the database is disconnected

@touilleMan
Copy link
Contributor Author

Updated PR to use gatherResults

@IlyaSkriblovsky
Copy link
Contributor

(I've deleted my comment abount gatherResults after noticed that it was in tests code and so optimization is not required ;) )

@touilleMan
Copy link
Contributor Author

Well now it's optimized ^^ !

@touilleMan
Copy link
Contributor Author

I've added fix from #161 to make this PR pass the unit tests

@touilleMan touilleMan mentioned this pull request Apr 28, 2016
Merged
@coveralls
Copy link

coveralls commented Apr 28, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-0.06%) to 95.623% when pulling 009a825 on touilleMan:issue-160 into 4ed6db0 on twisted:master.

IlyaSkriblovsky
IlyaSkriblovsky reviewed Apr 28, 2016
View reviewed changes
tests/test_objects.py
@@ -258,7 +265,9 @@ def test_GridFsObjects(self):
conn = yield txmongo.MongoConnection(mongo_host, mongo_port)
db = conn.test
gfs = GridFS(db) # Default collection
_ = yield gfs.get("test_3")
# Missing file raises error
with self.assertRaises(NoFile):
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, assertRaises() can be used as context manager only with Twisted ≥ 15 :(
Please rewrite it as yield self.assertFailure(gfs.get("test_3"), NoFile)

@touilleMan
Copy link
Contributor Author

I've changed the PR ;-)

@coveralls
Copy link

coveralls commented Apr 28, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-0.06%) to 95.623% when pulling 3341c2c on touilleMan:issue-160 into 4ed6db0 on twisted:master.

@IlyaSkriblovsky
Copy link
Contributor

Looks good, thanks!

@psi29a
Copy link
Contributor

psi29a commented May 2, 2016

👍

@psi29a psi29a merged commit cc762bc into twisted:master May 2, 2016
@psi29a psi29a added this to the 16.x - Getting things fixed milestone May 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
feature request
Projects
None yet
Milestone
16.x - Getting things fixed
Development

Successfully merging this pull request may close these issues.
4 participants
@touilleMan @IlyaSkriblovsky @coveralls @psi29a