Could we use typing.NamedTuple to make this easier to read?

class PipelineFailureSensorCursor(NamedTuple):
    record_id: int
    update_timestamp: str

    @staticmethod
    def is_valid(json_str):
        if json_str is None:
            return False
        try:
            obj = deserialize_json_to_dagster_namedtuple(json_str)
            return isinstance(obj, PipelineFailureSensorCursor)
        except JSONDecodeError:
            return False

    def to_json(self):
        return serialize_dagster_namedtuple(self)

    @staticmethod
    def from_json(json_str):
        return deserialize_json_to_dagster_namedtuple(json_str)

recend -> recent

Small thing, but it's a little bit dangerous to include a comment about how a function is implemented at a call site for that function - the implementation could change and the comment would end up inaccurate.

I don't think the parentheses around record_id, event_record are required.

It's a little bit weird to just ignore this parameter. I know it puts us in a tough spot, because if we honor it, we face the clock skew issue. Not sure what the best solution is - maybe the event log should expose some sort of boolean that expresses whether it can be queried by cursor?

Ah I see. That will raise an error though, not False, right? Because check errors are not JSONDecodeErrors?

Agree on the downside about that warning. @alangenfeld do you have an opinion on the private arg? I don't love it, but I'd be ok with it if you're OK with it.

ya i updated the except to capture all Exception. open to suggestion tho

should still feed mypy types using the less-nice NamedTuple set-up

what do you think about generalizing the cursor and allowing it flow through in after_cursor (and probably before_cursor)? EventsCursor, GlobalEventsCursor, AllEventsCursor

then each impl can unpack and discard/use what it wants and its contained within the impl

i think thats a brilliant idea! will do

should we have this and EventsCursor they seem kind of redundant (if we made EventCursor serdes)

think except (JSONDecodeError, CheckError) would cover all cases?

instead of CheckError I recently added a DeserializationError in the serdes module

PipelineFailureSensorCursor is the sensor's cursor while EventsCursor is a wrapper for storage cursor. At the moment they are kinda redundant (PipelineFailureSensorCursor does the serdes plus the validation and init) , but I'd prefer to separate these two to avoid future confusion.

If later we think they will be the same, we can drop this wrapper as it's just an internal representation.