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

What interface would we ideally want to expose for library users? #43

Closed
banksJeremy opened this issue Apr 30, 2014 · 6 comments
Closed

Comments

@banksJeremy
Copy link
Collaborator

Without thinking too much about the details of the implementation, I'm wonder what kind of interface would be nicest to expose to users of the library. I'm currently imagining something like:

import chatexchange

chat = chatexchange.connect( # class ChatConnection
    'chat.stackexchange.com', 'j@jeremybanks.ca', 'password')

me = chat.current_user # class User
assert me.user_name == "Jeremy Banks"

manish = chat.get_user(31768)
assert manish.user_name == "ManishEarth"

charcoal_sandbox = chat.get_room(14219) # class Room
assert charcoal_sandbox.name == "Charcoal Chatbot Sandbox"
assert manish in charcoal_sandbox.owners

def on_event(event): # class Event
    assert event.room == charcoal_sandbox
    assert event.room_id == charcoal_sandbox.room_id == 14219

    if event.type != event.Types.message_posted:
        return

    message = event.message # class Message
    # the Message object is shared between multiple events that refer
    # to it; e.g. a message_posted and following message_edited

    if message.text_body == "hello bot":
        charcoal_sandbox.send("thank you; goodbye user")

        watcher.stop()
        # this implicitly terminates the process because the watcher
        # was running the last remaining daemon thread.
    else:
        message.reply("please say `hello bot`")

watcher = sandbox.watch(on_event) # class RoomWatcher
# Watchers are abstracted from the underlying connections. You could
# create have several watchers that are being fed from the same
# socket or polling.

# This interface doesn't directly expose join/leaving rooms; the
# Connection will just ensure we're in a room if we're watching it, or
# if we need to talk in it. We could not worry about leaving rooms at
# this point.

Let me know if you have any thoughts.

@Manishearth
Copy link
Owner

So my initial idea was to do all this via SEChatWrapper, by extending the system we have now and adding methods as we go along.

But if we want a full API, this is probably necessary. I like your current template.

Will things like .owners (for example -- this can be any other property that requires a second request) be fetched when the room is initialized, or will they be fetched on demand via a property (getter)? Or should we allow for both -- let the room object lazily fetch them when required, but also allow for chat.get_room(1234, access = True) to tell it to scrape the access page beforehand?

@banksJeremy
Copy link
Collaborator Author

Yeah, I don't want to get too far ahead of myself, get too distracted by nice ideas, and lose focus on making the simple things actually work. But it's nice to have some idea of what we could provide later.

What you suggest is probably ideal: loading attributes lazily by default, with an option to fetch the data immediately.

Another possible interface for that behaviour would be to have a obj.loaded() method that requests all missing data for an object (a no-op if it's loaded), and then returns that object.

room = chat.get_room(1234, access=True) # all data explicitly requested

room = chat.get_room(1234).loaded() # all data explicitly requested

room = chat.get_room(1234)
room.room_name # room_name data implicitly requested

If it's exposed as a method instead of a parameter, users could request that an room's data be fetched at any point, not just at the point where they first retrieve the room object (small win). This also simplifies things if there are multiple ways to get room objects (bigger win):

For example, say we have a user.owned_rooms() method which returns a list of rooms. If we were using an access parameter, I guess we'd need to decide whether to retrieve the data from all or none of the rooms. If we instead use a method like loaded(), we would be able to do something like:

for room in user.owned_rooms():
    if room.room_id == 1:
        # we expect to be active in this room for some reason, so let's preload!
        room.loaded()

It would probably be reasonable to cache all user and room data during a session, so if you do chat.get_room(1234) when you've already accessed that room's data earlier,

That is, we ignore the possibility that room or user data changes after we initially get it. Later on, it might be nice to have some kind of automatic cache invalidation, so that the objects are automatically updated when you access a field that hasn't been reloaded in a few hours, but that's probably not a key feature.

@Manishearth
Copy link
Owner

Re: method vs parameter - we can just use an @property, puts the two at the same level :) We can then make .owner a "getter", and it can do the lazy loading.

My idea was like this, internally for each object, we have a bunch of lazy getter groups. For room, we can have info (loaded normally, contains recent messages and stuff), stars, access, schedule, etc. Each one exposes one or more property, and is a single HTTP call. We also have a .reload() to explicitly reload a group of properties.

The object stores a list of which groups have been loaded. If I ask for .owner, it will check if access has been loaded, and if not, it will load it. We probably can make this very easy to set up by using a bunch of decorators (Basically, we can prefix each getter with @lazy('access'), etc, and the rest of the magic is taken care of). This certainly would be fun to program :)

@Manishearth
Copy link
Owner

Basically, I envision something like this:

def init(self):
  self.gettergroups={"access":False,"info":False,...}
  self.info()
def access(self):
  # do some GETting
  # initialize related params

@lazy('access','owner')
@property
def owner(self):
  "NOP" #The decorator does the magic.
#etc etc

@banksJeremy
Copy link
Collaborator Author

Yeah, something like that sounds good.


I was just thinking that it would be nice to also have a synchronous way of consuming messages, rather than necessarily using a callback.

Maybe if you do room.watch(), without specifying a callback, it could instead start collecting all new events into an unbounded queue.

This might not be the easiest interface for a complicated bot (although you could have multiple watchers running from different threads if you wanted to), but for simple bots it could be convenient.

import chatexchange

chat = chatexchange.connect( # class ChatConnection
    'chat.stackexchange.com', 'j@jeremybanks.ca', 'password')

sandbox = chat.get_room(14219)

# context manager support to automatically close watcher when we're done with it
with sandbox.watch() as watcher:

    # an iterator over new messages - locked to ensure only one consumer
    for event in watcher.new_events():
        if event.type != event.Types.message_posted:
            continue

        message = event.message

        if message.text_body == "hello bot":
            charcoal_sandbox.send("thank you; goodbye user")

            break
        else:
            message.reply("please say `hello bot`")

If you call new_events() on a watcher that's using a callback instead of a queue, it should raise an error. If new_events() is locked to prevent access from multiple threads, there should also be a new_events(blocking=False) option which raises an error instead of blocking.

@Manishearth
Copy link
Owner

This seems doable. Not a priority (of course, feel free to implement it!), but it looks more pythony :)

This was referenced May 3, 2014
banksJeremy pushed a commit that referenced this issue May 14, 2014
        me = client.get_me()
        sandbox = client.get_room(11540)

        with sandbox.messages() as messages:
            sandbox.send_message("hello worl")

            for message in messages:
                if message.owner is me:
                    assert my_message.content == "hello worl"

ref #65, ref #52, ref #43
@banksJeremy banksJeremy self-assigned this May 15, 2014
banksJeremy pushed a commit that referenced this issue May 15, 2014
Moves some logic from client to room.

Adds simple docstrings with epydoc annotations to public fields and
methods of Client. Also adds epydoc as a dev dependency, and adds
`make epydocs`.

It probably isn't necessary to add this documentation to our internal
methods, and I won't bother with browser for now, but I think it's
useful to have for our user-friendly interface. (Both for users directly
and through IDEs that recognize the type annotations, like PyCharm CE.)

resolves #52 - let's forget about slugs and message sets for now. We can
reply to messages and a weak set of current messages might be pointless.

ref #65, #43 - event/message iterators/contexts are still outstanding.
banksJeremy pushed a commit that referenced this issue May 15, 2014
Moves some logic from client to room.

Adds simple docstrings with epydoc annotations to public fields and
methods of Client. Also adds epydoc as a dev dependency, and adds
`make epydocs`.

It probably isn't necessary to add this documentation to our internal
methods, and I won't bother with browser for now, but I think it's
useful to have for our user-friendly interface. (Both for users directly
and through IDEs that recognize the type annotations, like PyCharm CE.)

resolves #52 - let's forget about slugs and message sets for now. We can
reply to messages and a weak set of current messages might be pointless.

ref #65, #43 - event/message iterators/contexts are still outstanding.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants