search

bluesky / event-model

data model for event-based data collection and analysis
https://blueskyproject.io/event-model
BSD 3-Clause "New" or "Revised" License
15 stars 31 forks source link

Helpful warning makes development difficult #164

Closed jklynch closed 4 years ago

jklynch commented 4 years ago

A helpful warning was added to RunRouter.start(...) when its behavior was changed to pass the start document to each factory. Exceptions resulting from the new calls to callback('start', doc) trigger the warning and are squashed.

As I work on a new DocumentRouter subclass I wish the legitimate exceptions I am generating in start(...) would propagate all the way out and whack the interpreter just like all the other exceptions I generate.

Desired Behavior

I would like exceptions described above to be re-raised in addition to triggering the warning.

danielballan commented 4 years ago

Yes, I think our attempt to provide a gradual migration path was well-intentioned but flawed.

To give users the chance to defer updating the factories, we could add a kwarg to opt into the old behavior. I think given the low adoption of RunRouter so far we might instead just rush the deprecation cycle and release 1.15.0 with the change proposed here and no kwarg. Users who want the old behavior can stay on 1.14.x.

tacaswell commented 4 years ago

I have also come to like envs for controlling this sort of behavior. Very much like Python and gcc have options to turn warnings into errors, maybe we should have an ecosystem wide env (like BLUESKY_CRANKY, BLUESKY_FUTURE) that turns all deprecation warnings / sanding of edges into errors?

danielballan commented 4 years ago

OK, I agree that using an env variable here would be better than adding a kwarg. But I also think that "neither" is fine here.

jklynch commented 4 years ago

The issue I have with the current behavior is the assumption that an exception passing through these try/catch blocks is not a real exception and should be erased. I think this is "bad" behavior and should not be optional.

tacaswell commented 4 years ago

I agree in principle, but we also don't want to break working code with no warning / deprecation period.

jklynch commented 4 years ago

addressed by #166