Skip to content
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.

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

Readme improvements and a new version of Get #14

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,10 @@ queue = ironmq.queue("test_queue")
```python
queue.post("Hello world")
```
// Maybe a little more verbose here?

Message can be described by dict:
This message will have the default properties.
In order to customize them, Message can be described by dict:
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems a little weird to mix the proper noun (Message) with the common noun (the message). Standardising on one would probably be better.


```python
message = {
Expand All @@ -70,18 +72,52 @@ message = {
queue.post(message)
```

//
// For each of the above comments it would be nice to clarify when the countdown begins.
// For instance, for "expires_in" it's unclear whether this countdown restarts when a message is popped and then pushed back on the queue
Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, could definitely use some clarification.

// I suggest the following:


```python
message = {
"body" : "Test Message",
"timeout" : 120, # Timeout, in seconds. After timeout, item will be placed back on queue. Defaults to 60.
"delay" : 5, # The item will not be available on the queue until this many seconds have passed since message being pushed. Defaults to 0.
"expires_in" : 2*24*3600 # How long, in seconds, to keep the item on the queue before it is deleted. When a message is popped and then pushed back on the queue the countdown restarts.
}
queue.post(message)
```

We can post several messages at once:
```python
queue.post("more", "and more", "and more")
queue.post(*[str(i) for i in range(10)])
```

//
// All the example messages are strings: a user might think that only strings can be passed to "push" and "post" functions.
// If it's not so, adding an example with int/float would help avoid misunderstanding
Copy link
Contributor

Choose a reason for hiding this comment

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

Another good point. The docs should cover a reasonable representation of the capabilities.

//

### **Pop** a message off the queue:
```python
queue.get()
```
This will pop a message off the queue and return its body (aka a string)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure "aka" is the right term there. "As a" would probably work better.

In order to find out its attributes, use the following syntax:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a fan of "to find out". To retrieve, to obtain, to access would all be stronger choices.


```python
message = queue.get(verbose=True)
```

Now you have access to all the data associated with this message, i.e. message["timeout"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you mean e.g., not i.e.. i.e. stands for "id est", meaning "that is", and is used when offering a synonym or stand-in term. e.g. stands for "exempli gratia", meaning "for example", and is used when offering one of many examples. Because there is more than one piece of metadata associated with a message, e.g. would be the proper choice here.


When you pop/get a message from the queue, it will NOT be deleted.
It will eventually go back onto the queue after a timeout if you don't delete it (default timeout is 60 seconds).

// Stating constants several times throughout the manual is unsafe because if it gets altered
// an editor will have to change one statement and forget about the other one.
Copy link
Contributor

Choose a reason for hiding this comment

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

While this is true, and something we need to make a decision on, it's also true that using constants in a table or defining them elsewhere and just referring back to them forces the reader to bounce around to retrieve the information they want, interrupting the reading flow. It's a trade-off that needs to be made between ease-of-reading and ease-of-maintenance. I'm not sure we've struck the right balance, but I just wanted to point out that there are some other concerns with this, as well. Easy to maintain docs are useless if they're too hard to read.


### **Delete** a message from the queue:
```python
queue.delete(message_id)
Expand All @@ -104,9 +140,11 @@ queue.info()
queue.size() # 15
queue.name
queue.total_messages() # 17
queue.id() # u'502d03d3211a8f5e7742d224'
queue.id() # u'502d03d3211a8f5e7742d224'
```

// How can total_messages be greater than size?
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I'm mistaken (I may be), total_messages corresponds to the number of messages that have been sent on the queue since the queue was created. Size corresponds to the number of messages that remain on the queue. So after a message is deleted, total_messages would be greater than size.


# Full Documentation

You can find more documentation here:
Expand Down
8 changes: 6 additions & 2 deletions iron_mq.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,11 @@ def post(self, *messages):
return result['body']


def get(self, max=None):
def get(self, verbose=False, max=None):
"""Executes an HTTP request to get a message off of a queue.

Keyword arguments:
verbose -- If true, the entire dict is returned instead of the message body.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is definitely a handy feature. Not sure if "verbose" is the right keyword for it, but it is an elegant solution to the problem.

max -- The maximum number of messages to pull. Defaults to 1.
"""

Expand All @@ -99,7 +100,10 @@ def get(self, max=None):
n = "&n=%s" % max
url = "queues/%s/messages?%s" % (self.name, n)
result = self.client.get(url)
return result['body']
if verbose is True:
return result
else:
return result['body']


class IronMQ:
Expand Down