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

don't write cell.trusted to disk #5015

Merged
merged 1 commit into from Feb 4, 2014

Conversation

minrk
Copy link
Member

@minrk minrk commented Feb 4, 2014

Notary.check_cells should pop the keys, not just read them.

Notary.check_cells should pop the keys, not just read them.
return False
return True
if not cell.pop('trusted', False):
trusted = False
Copy link
Member

Choose a reason for hiding this comment

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

This now iterates over all cells, even if the first one isn't trusted. What was the problem with just returning immediately?

Copy link
Member Author

Choose a reason for hiding this comment

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

the 'trusted' key is written to disk

Copy link
Member

Choose a reason for hiding this comment

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

...that should have been obvious. Sorry.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, check_cells takes 50ms for 100k cells, so I wouldn't be too worried about iterating over the whole thing.

@takluyver takluyver added this to the 2.0 milestone Feb 4, 2014
takluyver added a commit that referenced this pull request Feb 4, 2014
@takluyver takluyver merged commit fdbdd53 into ipython:master Feb 4, 2014
@minrk minrk deleted the check-cells-pop-trusted branch March 31, 2014 23:36
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
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