Skip to content

global: delay the url_map building #5

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
Merged
merged 1 commit into from
Dec 11, 2015
Merged

global: delay the url_map building #5

merged 1 commit into from
Dec 11, 2015

Conversation

@krzysztof
Copy link
Member

@krzysztof krzysztof commented Dec 10, 2015

  • Delays the url_map building until first resolve request.

@krzysztof
Copy link
Member Author

krzysztof commented Dec 10, 2015

@jirikuncar Will this introduce any problems ?

I need this to build url_map where the path or host may contain config variables, e.g.: https://github.com/lnielsen/invenio-jsonschemas/blob/jsonresolver/invenio_jsonschemas/jsonresolver.py#L34-L43

@jirikuncar jirikuncar changed the title global: WIP delay the url_map building WIP global: delay the url_map building Dec 10, 2015
@jirikuncar jirikuncar added this to the v0.1.1 milestone Dec 10, 2015
@jirikuncar
Copy link
Member

jirikuncar commented Dec 10, 2015

@krzysztof it looks good. Please add label BETTER, sign your commit, add yourself to AUTHORS, and add a test-case that check this behavior (instantiate JSONResolver, check url_map to be None, resolve(), and url_map is not None). Thanks

update: even better check that your plugin was not called during JSONResolver.init

@krzysztof krzysztof changed the title WIP global: delay the url_map building global: delay the url_map building Dec 11, 2015
@krzysztof
Copy link
Member Author

krzysztof commented Dec 11, 2015

Ready
Python 3 problems
Ready

jirikuncar
jirikuncar reviewed Dec 11, 2015
View reviewed changes
tests/demo/raising_hook.py Outdated
Copy link
Member

@jirikuncar jirikuncar Dec 11, 2015

Choose a reason for hiding this comment

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

it probably needs isort

@krzysztof
Copy link
Member Author

krzysztof commented Dec 11, 2015

  • Fixed isort
  • Renamed exception
  • added top-level endpoint

@krzysztof
Copy link
Member Author

krzysztof commented Dec 11, 2015

Wait, this test is still wrong...

@krzysztof
Copy link
Member Author

krzysztof commented Dec 11, 2015

Ok, this one should test the lazy hook registration properly.

jirikuncar
jirikuncar reviewed Dec 11, 2015
View reviewed changes
tests/demo/raising_hook.py Outdated
Copy link
Member

@jirikuncar jirikuncar Dec 11, 2015

Choose a reason for hiding this comment

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

you can remove the dead code

@jirikuncar
Copy link
Member

jirikuncar commented Dec 11, 2015

@krzysztof it looks good now. Please remove dead code so we can :shipit: Thanks!

@jirikuncar jirikuncar assigned jirikuncar and unassigned krzysztof Dec 11, 2015
global: delay the url_map building
197ca0d 
* BETTER Delays the url_map building until first resolve request.

* Adds some missing docstrings.

Signed-off-by: Krzysztof Nowak <k.nowak@cern.ch>
@krzysztof
Copy link
Member Author

krzysztof commented Dec 11, 2015

Removed dead code

@krzysztof krzysztof mentioned this pull request Dec 11, 2015
Merged
@jirikuncar jirikuncar merged commit 197ca0d into inveniosoftware:master Dec 11, 2015
@krzysztof krzysztof deleted the lazy_url_map branch December 14, 2015 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@jirikuncar jirikuncar

Labels

Type: enhancement

Projects

None yet

Milestone

v0.1.1

Development

Successfully merging this pull request may close these issues.

2 participants

@krzysztof @jirikuncar