Skip to content
This repository has been archived by the owner on Oct 27, 2020. It is now read-only.

Current status of rat-board overhaul branch (work/dragons) #57

Closed
dewiniaid opened this issue Feb 3, 2016 · 2 comments
Closed

Current status of rat-board overhaul branch (work/dragons) #57

dewiniaid opened this issue Feb 3, 2016 · 2 comments

Comments

@dewiniaid
Copy link
Contributor

I'll be out of town for several days and won't be able to get the current code in my work branch up to even-remotely-stable by then (returning Sunday evening PST, probably Monday in actuality), so here's an update in case someone takes it on while I'm gone. This should probably be split into multiple issues, but I have too much to do IRL.

Deep magic at the end, basic stuff at the beginning

sopel-modules/rat-board.py

RescueBoard class

This replaces the various dicts and tracking mechanisms we've used so far with a more integrated management. Key features:

  • Can be used as a context manager (proxies to its lock controller). All access to a board sould be within its context manager, since it doesn't use SopelMemory().
  • Case number assignment now uses a hybrid available-id pool + an autoincrement in case of overflow.
  • Adding or removing a case with add/remove autopopulates various dicts that maintain an index of cases.
  • change() is a context manager for ensuring indexes update if they change on an existing case. !cmdr ... David Braben should at some point do with board.change(): rescue.client['CMDRname'] = 'David Braben'
  • find() is a unified method for finding (and possibly creating) rescues. It supports, in order from what is searched first.
    • Lookup by number. ("4" or "!close / !clear safety switch #4")
    • Lookup by API id by prefixing it with an @. ("@0248dead...beef0672")
    • Lookup by CMDR name and nickname, in that order.
      If create is True and find() doesn't find anything, it creates a case and sats the client CMDRname and nickname to whatever the search term was.

It's possible to create multiple RescueBoards. Among other uses, we can use an extra one as a recently closed cases queue, and one for a "special behavior in #drillrats" board that is isolated and disconnected from the API so drillspatch can use normal case management commands.

Rescue class

This represents an individual case. Has some convenience methods, and currently largely relies on ratlib/api/props.py (see Deep Magic at the end of this)

Overhaul

Starting from the top, I've been refactoring code to use the new models -- but I haven't reached the end of the file yet. Notable changes beyond the general refactor are:

  • append_quotes searches for (and possibly creates) a case, including filtering the line(s) through autocorrect (and any additional filters we might add). It returns a tuple of (rescue, newlines). newlines is useful to report what was actually added (i.e. the autocorrected system name, rather than the original), and rescue can be examined. (A rescue where id is None was newly created, vs added on to.)
  • Channel logging uses a collections.OrderedDict. Among other things, this sets an upper bound for how many distinct nicks the bot will maintain history for.
  • Interface to API calls was overhauled and moved to ratlib/api/http

Plan

In addition to finishing the overhaul, fixing bugs and finishing TODO:

  • When gathering changes to a Rescue to submit to the API, throw the resulting gathered changes into a queue. Objects in the queue are sent to the API in order (retrying as needed); if the API is down the changes will just queue indefinitely. Also, actually implement the gathering mechanism. (rescue.commit() is the planned mechanism for this, things that modify rescue should be able to mostly pretend the API doesn't exist.)
  • Save the state of the API submission queue (hopefully empty) and board on exit, so data can be recovered on startup if the API happens to be unavailable.
  • All the websocket stuff every, which should simplify a lot of the synchronization. (Query all relevant cases on startup, and then just listen for changes we care about).

Oh, huge chunks of this are at least partially tested in isolation, but not at a large scale. Like I said, I really ran out of time.

Known bugs

  • RescueBoard.change() doesn't lock.
  • RescueBoard._lock probably should be reentrant.
  • RescueBoard.find() should return immediately upon failure to find a case by ID or Number.

TODO

  • Implement functionality so Rescue can actually gather its changes and send them back to the API.

ratlib/api/http.py

Just a refactor and move of the code involved with API calls. Named 'http' in hopes of an eventual api/ws for websocket.

Oh, throw more detailed exceptions too.

ratlib/init.py

Adds a 'format_timedelta' function that returns exact results. Used for reporting how long a case has existed when it is closed, for giggles.

ratlib/api/props.py (HERE THERE BE DRAGONS)

This is a quick and dirty implementation of something that kind of mimics the change tracking of SQLAlchemy's ORM. Consider this a separate project from Mecha on par with something you might install with pip, so some portions of what it implements aren't relevant to us.

Properties and Metaclass

  • TrackedProperty is the basic property implementation, using Python's descriptor interface. Assignments to TrackedProperty are stored internally in a private _data dict on their instance, keyed to the property name. They also add themselves to the instance's _changed set, which allows one to examine an instance and see what properties have change. TrackedProperty has some framework for how the property should be converted to and from JSON as well. (Well, to/from a type that is JSON-serializable; they don't produce the JSON themselves.)
  • There are a few subclasses of TrackedProperty, used for some specific cases (like ensuring the Instrumented* properties remain the correct type even if they're blanket assigned.)
  • TrackedMeta is a metaclass. When a class uses this metaclass, any not-yet-named properties will be named based of their key in the class's namespace dict (so foo = TrackedProperty() in the class body yields a property named 'foo'), and the resulting TrackedProperty is added to the set cls._props
  • TrackedBase implements basic support for all of the above, and is intended to be the base class of anything doing change tracking. (Currently just Rescue, but this may see use elsewhere/in other projects).

Instrumented{List,Set,Dict}

These are subclasses of their corresponding Python base type. They do limited change tracking: when there's a clear way on how an operation should perform even if the instance's current state was different, they produce information on how to merge their current changes with a refreshed data source. When there's no clear way to resolve the conflict (basically, any modification that they can't handle), set their replaced property to True and just overwrite the new incoming data upon merge instead instead.

  • Lists remember their appended elements, whether through append or extend.
  • Dicts remember keys that were added, changed or deleted.
  • Sets remember what items were added and removed.

Changes to Instrumented objects that are not deterministic result will set the replace attribute to True, which means that it's interpreted as replacing any new data that comes from a refresh rather than reconciling, as do operations like obj.clear() or any other modification not supported.

The merge(other) method determines what to do if the underlying data is updated. The commit() method discards the change history and resets the replace -- intended to be called when we've successfully reconciled our changes with the other source (e.g. the API), like right after a merge.

Example: I want to assign Trezy to a case. I don't care who else is assigned. The case is currently stored in rescue, and `rescue.rats = {'Tyrope', 'Absolver'}

  1. I do rescue.rats.add('Trezy'). Behind the scenes, this adds a dict entry {'Trezy': True}, where the True indicates that it was an addition rather than a deletion.
  2. Before writing the change, Mecha refreshes the case. Looks like Burwellian was added too.
  3. rescue.rats.merge(refreshed_rats) checks to see if it needs to do a full replace. Since it doesn't, it clears itself, adds everything in rescue_rats, and then adds everything in its own list of additions. rescue.rats now equals {'Tyrope', 'Absolver', 'Burwellian', 'Trezy'}

Example 2: I royally botched up and assigned a bunch of rats to the wrong case. So I want to clear who's assigned.

  1. I do rescue.rats.clear() Internally, the replace property is set to True.
  2. A refresh happens.
  3. replace is true, so the refresh is ignored, and I send whatever my current rescue.rats value is.

Known Bugs

  • Instrumented classes don't have a way of notifying their parent property that they've changed. This is one of the major reasons for having them, but should be an easy fix.
  • Assigning to something that should have a instrumented class won't trigger replacement, though it will be coerced to the correct type. This means that rescue.rats = set() and rescue.rats.clear() do not yield the same result if data is merged in.
@dewiniaid
Copy link
Contributor Author

Updates:

  • Fixed known bugs with api.props, so Rescue knows which fields have been modified:
  • Ditched the framework for retrying API calls for now for simplicity's sake.
  • Ditched framework for refresh+merge+commit workflow for simplicity's sake. Websockets will make this largely unnecessary in the long run anyways.
  • Reimplemented most commands under the new framework. Notable omissions are assign/unassign and new commands like cmdr/op/etc.
  • When a case is created and the line(s) match "pc" on a word boundary, the platform is automatically set to PC. When they match xb(ox|one|1) on a word boundary, the platform is automatically set to Xbox. This may result in false detections, but they should be minimal and can still be set with !pc/!xbox.

@dewiniaid
Copy link
Contributor Author

Current work/dragons should be fully functional minus the assign bits, which are waiting on new API functionality.

If detached from the API, it should be functional as-is.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants