This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2596

state: raise ProgrammingError if an invalid key is being accessed
AbandonedPublic

Authored by pulkit on Mar 3 2018, 2:19 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Mar 3 2018, 2:19 PM
durin42 added a subscriber: durin42.Mar 4 2018, 11:13 AM

Hmm, really? Do any states have optional entries that would make this awkward?

pulkit added a comment.Mar 4 2018, 11:15 AM
In D2596#42672, @durin42 wrote:

Hmm, really? Do any states have optional entries that would make this awkward?

Nope, but I wanted to be safe here raising a ProgrammingError instead of KeyError.

yuja added a subscriber: yuja.Mar 4 2018, 12:47 PM
In D2596#42678, @pulkit wrote:
In D2596#42672, @durin42 wrote:

Hmm, really? Do any states have optional entries that would make this awkward?

Nope, but I wanted to be safe here raising a ProgrammingError instead of KeyError.

I don't follow, but what if the state file was written by old hg client where some new keys
were missing, and read by new client? Perhaps we'll have to support that scenario.

mercurial/state.py
61

Nit: no need to make a ProgrammingError translatable.

pulkit added a comment.Mar 4 2018, 3:50 PM
In D2596#42818, @yuja wrote:
In D2596#42678, @pulkit wrote:
In D2596#42672, @durin42 wrote:

Hmm, really? Do any states have optional entries that would make this awkward?

Nope, but I wanted to be safe here raising a ProgrammingError instead of KeyError.

I don't follow, but what if the state file was written by old hg client where some new keys
were missing, and read by new client? Perhaps we'll have to support that scenario.

I am going to drop this patch for now so in cases which Yuya mentioned, we can catch the KeyError to realize that key is not available.

pulkit abandoned this revision.Mar 19 2018, 4:45 AM

Revision Contents
Changeset List

Diff 6447

mercurial/state.py

self.opts = {} self.opts = {}
else: else:
self.opts = opts self.opts = opts
def __nonzero__(self): def __nonzero__(self):
return self.exists() return self.exists()
def __getitem__(self, key): def __getitem__(self, key):
try:
return self.opts[key] return self.opts[key]
except KeyError:
raise error.ProgrammingError(_("key '%s' not found in state"
yujaUnsubmitted
Not Done

Nit: no need to make a ProgrammingError translatable.

yuja: Nit: no need to make a ProgrammingError translatable.
" file %s") % (key, self.path))
def load(self): def load(self):
"""load the existing evolvestate file into the class object""" """load the existing evolvestate file into the class object"""
op = self._read() op = self._read()
self.opts.update(op) self.opts.update(op)
def addopts(self, opts): def addopts(self, opts):
"""add more key-value pairs to the data stored by the object""" """add more key-value pairs to the data stored by the object"""