models: initial sequence implementation #6
Conversation
|
@omelkonian please try to be gentle with opening new PRs. You can always reopen and re-push to the same branch if you need. |
jirikuncar
changed the title
| template = db.Column(db.String, unique=True) | ||
| """The template of the sequence.""" | ||
|
|
||
| parent = db.Column(ForeignKey(name)) |
|
@jirikuncar I am currently implementing the admin view. Should I do a separate commit on this PR or a separate PR altogether? |
|
@omelkonian it's better to do each feature in separate branch so we don't discuss about admin interface here when the models are good. |
|
@omelkonian please add tests to keep the test coverage. Thanks! |
| class NoSuchSequence(Exception): | ||
| """No such sequence error.""" | ||
|
|
||
| pass |
There was a problem hiding this comment.
You don't need a pass if there is a docstring.
|
@jirikuncar Just to clarify, the PRs should be merged in this order: #7 => #6 => #8 |
|
@jirikuncar Revived API is up. |
| COUNTER_REGEX = re.compile(r'({counter(!.)?(:.*)?})') | ||
| """Regular expression matching the counter inside the template string.""" | ||
|
|
||
| id = db.Column(db.String, primary_key=True) |
|
@jirikuncar @egabancho Please check updated version. I think it much clearer now :) |
|
|
||
| :param name: the identifying name of the sequence | ||
| :param template: the template string of the sequence | ||
| :param start: the starting counter of the sequence |
There was a problem hiding this comment.
move the docs to create method
|
@jirikuncar Please wait for docstrings update (due to renaming) before merging. |
| def __init__(self, name, _model=None): | ||
| """Initialize template. | ||
|
|
||
| :param name: the identifying name of the template |
There was a problem hiding this comment.
Please start all param descriptions with capital letter and finish with a dot. Thanks
|
@jirikuncar I will add the repr methods with the admin PR. |
|
@omelkonian please rebase and we can |
jirikuncar
added
Status: in review
Type: new feature
Size: hard
and removed
Status: in work
Need: tests
labels
|
ping @jirikuncar @egabancho |
| from __future__ import absolute_import, print_function | ||
|
|
||
| from invenio_db import db | ||
| from invenio_sequencegenerator.errors import SequenceNotFound |
There was a problem hiding this comment.
can we use relative imports like in the rest of the package?
|
LGTM |
* Added model for defining auto-incrementing sequences. * Added convenient API using iterators. * Features include user-defined keywords and hierarchical sequences. * Added tests for complete coverage. Signed-off-by: Orestis Melkonian <melkon.or@gmail.com>
Signed-off-by: Orestis Melkonian melkon.or@gmail.com