Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

History section of messaging spec is incorrect #402

Closed
epatters opened this Issue Apr 26, 2011 · 6 comments

Comments

Projects
None yet
3 participants
Contributor

epatters commented Apr 26, 2011

The section of the messaging spec describing the history protocol is outdated.

Are we dropping 'history_request' in favor of the (less flexible) 'history_tail_request'? If so, we need to change the spec. If not, we should implement the full protocol using Thomas' new history manager.

Owner

takluyver commented Apr 28, 2011

Ah, yes, that was me. I wasn't really aware there was a spec when I changed it, I was just making it work.

I didn't like the way that the get_history method did a handful of different things depending on its input, so I split it up into three methods: get_range, get_tail and search. I don't know if it makes more sense to have a different request type for each, or unify them under a history_request with a parameter to specify what kind of request is being made.

Contributor

epatters commented Apr 28, 2011

I prefer a single, unified history_request myself, but I don't have a very strong opinion about this.

Let's see what @fperez and @ellisonbg think, since they were principally responsible for writing this part of the spec.

Owner

ellisonbg commented Apr 28, 2011

Histories can get really big and since this involves network traffic, I like the idea of having more fine grained history recall mechanisms.

Owner

takluyver commented May 3, 2011

I think the granularity and network traffic will be about the same. It's a semantic question of whether we have history_tail_request, history_range_request, history_search_request, or lump them all into history_request and use a parameter (hist_request_type?) to distinguish them. I think I'd lean towards a single history_request, although I don't have a strong feeling either way.

Owner

takluyver commented May 3, 2011

See pull request #408.

@takluyver takluyver was assigned May 3, 2011

Owner

takluyver commented May 11, 2011

Closed by merge of PR #408.

@takluyver takluyver closed this May 11, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment