Skip to content
This repository has been archived by the owner on Jul 24, 2018. It is now read-only.

Create a new /info endpoint #9

Merged
merged 6 commits into from Jan 17, 2017
Merged

Create a new /info endpoint #9

merged 6 commits into from Jan 17, 2017

Conversation

pypingou
Copy link
Member

This endpoint returns information about the number of sessions total and active associated
to this IP.


output['total_connections'] = model.Connection.search(
session, ip=ipaddr, cnt=True)
output['active_connections'] = model.Connection.by_ip(
Copy link
Member

Choose a reason for hiding this comment

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

Why not use search for both queries or by_ip for both queries? They seem to do the same thing in this case.

Edit: In all cases, actually

This endpoint aims at returning information about the session attached
to the current IP.
Both method were basically doing the same thing now, so let's just keep
one of them, as pointed out by Jeremy Cline in the review
@pypingou
Copy link
Member Author

Good call, I just dropped by_id() and replaced it with calls to search() instead.

Thanks :)


{% if ip in config['IP_UNLIMITED'] %}
This IP can have un-limited sessions.
{% elif ip in config['IP_UNLIMITED'] %}
Copy link
Member

Choose a reason for hiding this comment

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

I think maybe this was meant to be 'IP_BLOCKED' or similar?

Copy link
Member Author

Choose a reason for hiding this comment

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

That sounds right :D

@jeremycline
Copy link
Member

Other than the one kwarg which I think is wrong, this looks good 👍

@pypingou
Copy link
Member Author

I'm going to merge this, thanks for the review :)

@pypingou pypingou merged commit f20b49c into master Jan 17, 2017
@pypingou pypingou deleted the info_ip branch January 17, 2017 09:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants