> This way you could > maintain important info from the beginning of the stack all the way to > end of the stack. Filters that are aware of the envelope can use it, > otherwise, it is just another arrayref with records. cant it just be a blessed arrayref blessed to a singleton class that has a single method ( fetch_error() ) that retrieves the error obj/scalar/string/num/whatever ?
On 9/5/07, Nicholas Perez <[EMAIL PROTECTED]> wrote: > I think this is a very sane approach. My only comment would be to make > the Datatype class named to something else, such as what it really is: > Envelope. Then the Envelope itself can contain properties such as if > it contains an error or whatever. How we do dynamic properties > without affecting the contents of the arrayref (closure magic?), I > don't know, but that is just detail at this point. This would allow us > to bind the in-band and out-of-band data together. This way you could > maintain important info from the beginning of the stack all the way to > end of the stack. Filters that are aware of the envelope can use it, > otherwise, it is just another arrayref with records. > > On 9/5/07, Rocco Caputo <[EMAIL PROTECTED]> wrote: > > Yay, my outbound e-mail works again! I can comment on this thread now! > > > > I require three things (today... maybe more, or others tomorrow): > > > > 1. Maintain backwards compatibility. Old and new filters should > > interoperate. Require the least amount of collateral code change > > possible. Most of the burden of this feature is implemented in the > > filters themselves. > > > > 2. Filters must work stand-alone. This is a great feature, allowing > > people to write thin, blocking clients to POE servers. The technique > > is documented in at least one cookbook example. > > > > 3. Filter status notifications must remain in-band. That is, it > > should be passed through filters along with regular data. This > > should resolve a potential race condition between the last data in > > some stream and the EOF itself. Currently EOF is delivered > > immediately upon the driver noticing it. If any data remains in the > > filter pipeline, it's lost. > > > > The typed container idea works for me. If we require the get_one() > > interface, it already returns an untyped container: the list > > reference that envelopes the message. Normal data can leave the > > envelope type unblessed. Various statuses can be defined by > > POE::Filter::Datatype subclasses and bound to the envelope with bless(). > > > > There would need to be a way for external glue code (POE::Wheel::* > > and POE::Filter::Stackable, mainly) to tell whether a filter supports > > in-band notifications, without modifying the filters that don't. > > Possibly some accessor that's present on the newer filters. > > > > In-band notifications would require filter buffers to be list-based. > > Notifications must remain separate from data within the filters. > > Some filters, like POE::Filter::Stream, have scalar-based buffers. > > > > In-band notifications would require the get_one() interface. get_one > > () returns data wrapped in superfluous listref envelopes. We can > > bind type information to that envelope without touching the messages > > that pass through the filter. > > > > The API for get_pending() may need to change. :( > > > > Filter::Stackable would check each filter in the stack. Status > > notifications would skip filters that don't support them. > > > > POE::Filter::Datatype types would not be exposed to the application. > > They are only used internally by POE::Wheel::* and the filters. > > Convert them to "plain" messages (move the status inside the > > envelope, and clear the envelope type) if you want to expose them to > > the user. Or wheels can catch them and do something with them. For > > example, POE::Filter::Datatype::EOF would be converted to an > > ErrorEvent by POE::Wheel::ReadWrite. It would become a ClosedEvent > > in POE::Wheel::Run. > > > > Some sample code, adapted from POE::Filter::Stream and not at all > > tested: > > > > sub new { > > my $class = shift; > > return bless [ ], $class; > > } > > > > sub supports_inband_status { 1 } > > > > sub get_one_start { > > my ($self, $stream) = @_; > > > > # If the $stream envelope has a special data type, then > > # distribute that across all the messages in the envelope. > > > > if (ref($stream) =~ /^POE::Filter::Datatype::/) { > > push @$self, map { [ ref($stream), $_ ] } @$stream; > > return; > > } > > > > # Otherwise it's plain data. Combine the streamed data, > > # and buffer it. > > > > if (@$self and ref($self->[-1]) eq "ARRAY") { > > $self->[-1] .= join "", @$stream; > > return; > > } > > > > push @$self, join "", @$stream; > > return; > > } > > > > # Return the next chunk, which was prepared in get_one_start(). > > > > sub get_one { > > my $self = shift; > > return bless [ ], "POE::Filter::Datatype::Empty" unless @$self; > > return shift @$self; > > } > > > > # Return a list rather than a list reference. > > # The problem here is that we may be returning > > # many different types of things, each requiring > > # its own envelope. > > > > sub get_pending { > > my $self = shift; > > return @$self; > > } > > > > -- > > Rocco Caputo - [EMAIL PROTECTED] > > > > > > On Aug 15, 2007, at 11:57, Nicholas Perez wrote: > > > > > The only reason I questioned it is because I heard mentioned in #poe > > > about wanting to maintain the non-POE-ness of filters (ie. usage > > > outside of POE). I could have the context wrong, though and I forgot > > > specifically who said it? Chris maybe? > > > > > > On 8/15/07, Matt Sickler <[EMAIL PROTECTED]> wrote: > > >> How is adding that error mechanism binding the filter to the wheel > > >> any > > >> more than the current get_one() and put() API is? > > >> > > >> I don't care how many API's we have to break, Filters should have > > >> proper error checking. > > >> Sure there are alot of third party filters, but 90% of the devs > > >> are on > > >> #poe, and the other 10% are probably smart enough to read the docs > > >> and > > >> figure out the changes. > > >> > > >> On 8/15/07, Nicholas Perez <[EMAIL PROTECTED]> wrote: > > >>> This certainly looks promising, but how do you know when to read it? > > >>> When the filter returns an empty array ref? I'd like not for the > > >>> error > > >>> checking to be required after every time an empty array ref gets > > >>> returned, because that could be quite a few times depending on > > >>> how big > > >>> of a buffer the filter has before a usable chunk comes out the other > > >>> end. Also, it seems like the OOB stuff shouldn't get passed in to > > >>> get_one(). While I'm all for breaking the API to fix something, it > > >>> seems like there should be an OOB mechanism built-in, that doesn't > > >>> require passing arguments to get_one. > > >>> > > >>> I still thinking adding a property is the way to go. To indicate > > >>> error > > >>> state, I like the idea of passing into the constructor a ref to a > > >>> scalar and checking its boolean value after each execution (I think > > >>> this sounds like a compromise). That seems like less cycles for > > >>> error > > >>> checking, and we don't change the return [ ] behavior on error or no > > >>> records. > > >>> > > >>> The only problem with an error indicator passed in is this: we have > > >>> just coupled the container to the filter (as witnessed by the > > >>> changes > > >>> to readwrite). Do we really want this? > > >>> > > >>> In reality any of these approaches will work. I would just like to > > >>> find the least painful of them all. > > >>> > > >>> On 8/14/07, David Davis <[EMAIL PROTECTED]> wrote: > > >>>> My work on POE::Filter::SSL is relevant here. > > >>>> > > >>>> I needed a way to pass OOB data back to the readwrite wheel > > >>>> during the SSL > > >>>> negotiation. The method LotR and I came up works great. I pass > > >>>> an array > > >>>> ref to get_one (lets call it $oob), and any time a filter has > > >>>> OOB data, it > > >>>> blesses it into a namespace, and pushes it onto @$oob. For > > >>>> Filter::SSL, I > > >>>> used the OOB namespace 'POE::OOB::put' for data that needs to be > > >>>> directly > > >>>> sent out the wheel's driver. Filter::Stackable and > > >>>> Wheel::ReadWrite needs > > >>>> to be aware of this OOB data, so I've patched them too. We can > > >>>> use this > > >>>> method to add 'POE::OOB::error' and handle it by sending an event. > > >>>> > > >>>> Relevant files: > > >>>> http://svn.xantus.org/sprocket/trunk/Sprocket/lib/POE/Filter/ > > >>>> Stackable.pm > > >>>> http://svn.xantus.org/sprocket/trunk/Sprocket/lib/POE/Filter/SSL.pm > > >>>> http://svn.xantus.org/sprocket/trunk/Sprocket/lib/POE/Wheel/ > > >>>> ReadWrite.pm > > >>>> > > >>>> Cheers, > > >>>> David > > >>>> > > >>>> On 8/14/07, Martijn van Beers <[EMAIL PROTECTED]> wrote: > > >>>>> > > >>>>> There was a discussion about this on #poe, which I thought I'd > > >>>>> share: > > >>>>> > > >>>>> <+nperez> an isa check every single time something goes through > > >>>>> the > > >>>>> filter just seems like overkill... > > >>>>> <+LotR> compared to the work that Filter::XML does? > > >>>>> <+nperez> yes, because it will add up. hold on, writing a > > >>>>> benchmark > > >>>>> script > > >>>>> <+LotR> I'm sure it will prove to be nothing compared to > > >>>>> parsing an xml > > >>>>> subtree > > >>>>> <+nperez> but what about other filters? Stream? Line? > > >>>>> <+LotR> stream doesn't ever need to generate any errors. nor > > >>>>> does line > > >>>>> <+LotR> if you just use those, you can forget about error checking > > >>>>> altogether > > >>>>> * LotR will send the discussion to the list when we're done > > >>>>> <+nopaste> "nperez" at 12.5.72.252 pasted "undef vs isa" (30 > > >>>>> lines) at > > >>>>> http://nopaste.snit.ch:8001/10886 > > >>>>> <+nperez> take a look > > >>>>> <+nperez> that's a huge difference > > >>>>> <+LotR> but an isa() still is pretty damn fast. > > >>>>> <+nperez> besides, it is less of a rewrite checking for undef. > > >>>>> consider: > > >>>>> you have to now check ref and then isa on every > > >>>>> return value > > >>>>> from the filter > > >>>>> <+nperez> what if it isn't a ref that gets returned? > > >>>>> <+nperez> and isa only worked on blessed objects > > >>>>> <+LotR> you're still going to have to check for undef too > > >>>>> <+nperez> works > > >>>>> <+nperez> exactly, so why do the other checks and you can just > > >>>>> do the > > >>>>> one > > >>>>> <+LotR> and it is going to be more of a change if you then have > > >>>>> to ask > > >>>>> the filter for extra data > > >>>>> <+nperez> not really, implementing an get_error() isn't as > > >>>>> profound of > > >>>>> a > > >>>>> change > > >>>>> <+nperez> it is OOB > > >>>>> <+nperez> without you still have checking for undef for errors > > >>>>> in a lot > > >>>>> of modules anyhow > > >>>>> <+nperez> if they want to know more info, they then call > > >>>>> get_error() > > >>>>> <+nperez> otherwise, they might not care > > >>>>> <+LotR> that "whout you still.." sentence doesn't parse for me > > >>>>> <+LotR> without. duh > > >>>>> <+nperez> sorry, without get_error(), you still have checking > > >>>>> for undef > > >>>>> <+LotR> still don't get it. how does checking for undef for > > >>>>> errors in > > >>>>> 'a > > >>>>> lot of modules' have anything to do with filters? > > >>>>> <+nperez> because right now, the de facto indicator for error is > > >>>>> returning undef > > >>>>> <+LotR> no it isn't > > >>>>> <+LotR> in Filter::HTTPD, an HTTP::Response is returned > > >>>>> <+LotR> in Filter::XML, you get a callback > > >>>>> <+LotR> in Filter::SAXBuilder, you get an Error::Simple object > > >>>>> <+LotR> and I don't know any other filters that return errors > > >>>>> right now > > >>>>> <+nperez> okay, thinking a little more general, perl built-ins > > >>>>> return > > >>>>> undef on errors and set $! > > >>>>> <+nperez> following that convention can't be /that/ bad > > >>>>> <+LotR> right, but is it guaranteed that you always get the > > >>>>> chance to > > >>>>> check the error before an extra error is created (meaning > > >>>>> filters > > >>>>> have to store more than one, and the $! analogy breaks > > >>>>> down) > > >>>>> <@Leolo> test it against Scalar::Util::blessed > > >>>>> <+nperez> Leolo: that is a possibility too > > >>>>> <+LotR> Leolo: hmm? > > >>>>> <@Leolo> also, the filter could return undef(), then you ask > > >>>>> the filter > > >>>>> ->is_error() or ->error_str() or whatever > > >>>>> <@Leolo> call this wrapped in POE::Filter::Fallible > > >>>>> <+LotR> I just don't particulary like the extra API > > >>>>> <@Leolo> s/call/all/ > > >>>>> <+nperez> well, it's incomplete as is. Error states are simply > > >>>>> unaccounted for. Most of the modules that I have been > > >>>>> looking > > >>>>> through in the past 10 minutes make all sorts of > > >>>>> assumptions > > >>>>> on > > >>>>> input > > >>>>> <+nperez> if there is an error, it will just die > > >>>>> <+nperez> or return [ ] > > >>>>> <+nperez> in which case, how do you know there was an error? > > >>>>> <+LotR> you don't. But I like the simplicity of array ref in; > > >>>>> array ref > > >>>>> out > > >>>>> <@Leolo> they probably assume they are talking to another > > >>>>> POE::Filter::Foo. in which case there should be no error > > >>>>> <@Leolo> lotr : true > > >>>>> <+nperez> the simplicity fails. If the filter gets into an > > >>>>> inconsistent > > >>>>> state due to bad input, but never tells the outside > > >>>>> world > > >>>>> what > > >>>>> is going on, that's just poor > > >>>>> <+nperez> returning [] is just too overloaded > > >>>>> <+LotR> plus, if you stack the filters, can you make sure you > > >>>>> will just > > >>>>> have had one error if you get out of the filter? > > >>>>> <+LotR> nperez: I'm not saying returning [] on error is a good > > >>>>> idea, it > > >>>>> isn't. > > >>>>> <+nperez> there probably needs to be a way to aggregate/bubble > > >>>>> errors > > >>>>> up > > >>>>> from stacked filters > > >>>>> <+LotR> I'm saying returning an error object in the array ref > > >>>>> out keeps > > >>>>> the simplicity > > >>>>> < imMute> maybe it should return some kind of > > >>>>> POE::Filter::Error object > > >>>>> on error > > >>>>> < imMute> and then the wheel thats processing it can deal with > > >>>>> the error > > >>>>> < imMute> or the next filter in stackable can just pass it on > > >>>>> <+nperez> LotR: but that adds checking in-band which is a > > >>>>> performance > > >>>>> killer > > >>>>> <+nperez> maybe no killer, but detrimental to performance i > > >>>>> should say > > >>>>> <+nperez> maybe no killer, but detrimental to performance i > > >>>>> should say > > >>>>> < imMute> its better than just suddenly not getting any records > > >>>>> from a > > >>>>> filter and not knowing why > > >>>>> <+LotR> nperez: right. so we disagree on how bad such a > > >>>>> performance hit > > >>>>> that is > > >>>>> <+nperez> imMute: getting an undef should be a strong idicator > > >>>>> of error > > >>>>> in which case you check the error property on the filter > > >>>>> <+nperez> agreed on that point LotR > > >>>>> < imMute> nperez: are we going to standardize the name of that > > >>>>> call, > > >>>>> and > > >>>>> what it should return? > > >>>>> * LotR wonders if there are no filters that return undef for > > >>>>> reasons > > >>>>> other than error > > >>>>> <+nperez> imMute: that's what we would like to happen, yes > > >>>>> < imMute> personally I think we should standardize the arrayref > > >>>>> in, > > >>>>> arrayref out first, because I know there are filters > > >>>>> that > > >>>>> dont > > >>>>> follow that > > >>>>> <+nperez> it is standard, if they subclass POE::Filter > > >>>>> <+nperez> and stay "true" to form > > >>>>> < imMute> some dont though > > >>>>> <+nperez> then we should file bugs on rt for them :) > > >>>>> < imMute> i know few of BinGOs' filters subclass POE::Filter > > >>>>> < imMute> and he complained loudly when i added an isa() check to > > >>>>> Stackable > > >>>>> <@BinGOs> all of them do. > > >>>>> <+nperez> I did too > > >>>>> <+nperez> I had to make changes to make the filter stabled > > >>>>> <+nperez> stackable > > >>>>> <+purl> well, stackable is newer than PFX? > > >>>>> <@BinGOs> this is why I was screaming blue fucking murder earlier. > > >>>>> < imMute> maybe its changed since i last messed with filters, > > >>>>> its been > > >>>>> a > > >>>>> few months > > >>>>> <+nperez> It is implicit that filters take aref in and spit > > >>>>> aref out > > >>>>> <+LotR> explicit in the docs actually > > >>>>> <+nperez> right > > >>>>> <@BinGOs> My take is POE::Filter shouldn't shit out anything they > > >>>>> shouldn't > > >>>>> <+LotR> BinGOs: meaning? > > >>>>> <+purl> somebody said meaning was in the individual, whether a > > >>>>> definition > > >>>>> <@BinGOs> otherwise a world of fucking hurt is going to happen. > > >>>>> <@BinGOs> meaning, don't return anything on a fucking error. > > >>>>> <+LotR> why not? > > >>>>> <@BinGOs> because I will hunt you down like a dog > > >>>>> <+nperez> BinGOs: if there is an error, I'd like to know about it > > >>>>> <@BinGOs> then 'warn' > > >>>>> <+nperez> returning [] is not tenable > > >>>>> <+nperez> the state of the filter may need to be reset > > >>>>> <+nperez> I can't capture warns and adjust accordingly > > >>>>> <@BinGOs> if you are in a POE::Filter::Stackable then everyone > > >>>>> may not > > >>>>> be > > >>>>> following the rules. > > >>>>> <+nperez> which is why we are wanting to update the POE::Filter > > >>>>> API and > > >>>>> introduce a standard way to report errors > > >>>>> <+LotR> BinGOs: there'll be new rules :) > > >>>>> <+nperez> heh > > >>>>> <@BinGOs> I am not the bad guy here >:) > > >>>>> <+nperez> BinGOs: if you want patches to your modules after the > > >>>>> change, > > >>>>> I'll help, promise. Because you have a shit load and > > >>>>> I don't > > >>>>> blame you for beign resistant to change ;) > > >>>>> <+LotR> BinGOs: oh no? so what happens if Filter::IRC gets a > > >>>>> line that > > >>>>> doesn't parse? > > >>>>> <@BinGOs> it warns > > >>>>> <+LotR> which isn't very nice > > >>>>> <@BinGOs> and b). I am not using POE::Filter::IRC these days. > > >>>>> <+LotR> Filter::IRCD then > > >>>>> <@BinGOs> and c). yes, POE::Filter::IRCD warns > > >>>>> <+nperez> warnings disappear into the aether and don't control > > >>>>> autocorrecting procedures > > >>>>> <+LotR> BinGOs: which you can't handle very nicely. > > >>>>> <@BinGOs> Okay, you have a POE::Filter::Stackable > > >>>>> < imMute> what does PCI use for the client then? > > >>>>> <@BinGOs> the first filter produces an error, what happens next ? > > >>>>> <@BinGOs> ( you have four filters in the chain ) > > >>>>> <+nperez> stackable checks the error property, sets its own error > > >>>>> property and returns undef > > >>>>> <+nperez> skipping the other three > > >>>>> <@BinGOs> do you produce an event at the end ? > > >>>>> <+LotR> or > > >>>>> <+LotR> it sticks an error object in the listref out. (and > > >>>>> probably > > >>>>> skips > > >>>>> the other three filters) > > >>>>> < imMute> BinGOs: no > > >>>>> <+LotR> BinGOs: filters don't know anything about POE events. I > > >>>>> want to > > >>>>> keep it that way > > >>>>> <@BinGOs> PCI uses Filter::IRCD and POE::Filter::IRC::Compat in a > > >>>>> stackable. > > >>>>> < imMute> BinGOs: stackable doesnt produce output unless the last > > >>>>> filter > > >>>>> returns something > > >>>>> <@BinGOs> SO what happens to your error then ? > > >>>>> <+nperez> returning undef should be an indicator to check the > > >>>>> error > > >>>>> property on the filter > > >>>>> <+LotR> BinGOs: read the scrollback? we've been over this and > > >>>>> haven't > > >>>>> come to an agreement. nperez and I have different views > > >>>>> on how > > >>>>> to > > >>>>> do it > > >>>>> <+LotR> but we agree there should be some error reporting in > > >>>>> filters > > >>>>> <@BinGOs> Hell, I'll work around whatever you come up with > > >>>>> anyways >:) > > >>>>> <+nperez> haha > > >>>>> <@BinGOs> difficult audience. > > >>>>> <+nperez> Ultimately is up to dngor about an API change > > >>>>> <+nperez> we are wanting to do something rather fundamental > > >>>>> <+nperez> especially with a 1.0 approaching > > >>>>> <@BinGOs> this is why I am stomping my little feet. > > >>>>> <+nperez> If it's broken, it's broken. Bitching how much the > > >>>>> fix hurts > > >>>>> isn't going to make it go away :) > > >>>>> <+LotR> another reason not to do an ->get_error() method. no > > >>>>> need to > > >>>>> change old filters who don't care about error reporting > > >>>>> <+nperez> if they don't care about error reporting, they aren't > > >>>>> checking > > >>>>> their inputs enough ;) > > >>>>> <@BinGOs> I am a team player and have a 'Can do attitude' > > >>>>> <@BinGOs> the whole POE::Filter sub-class thing was no problem > > >>>>> to me. > > >>>>> <+nperez> BinGOs: we aren't interviewing you here. You are more > > >>>>> than > > >>>>> welcome to be the devil's advocate and general > > >>>>> asshole. It > > >>>>> helps :) > > >>>>> <@BinGOs> Yes, I am playing devil's advocate > > >>>>> <@BinGOs> and yes, I am an asshole. > > >>>>> <+nperez> You represent a large swath of incumbent modules. Your > > >>>>> opinion > > >>>>> counts > > >>>>> <@BinGOs> I always have an eye on backwards compatibility. > > >>>>> <@perigrin> to watch it slip away > > >>>>> <+nperez> That is certainly something to consider. LotR's idea is > > >>>>> probably better for backwards compat > > >>>>> <@Leolo> backwards compatible with "do nothing useful" is easy > > >>>>> > > >>>>> > > >>>>> > > >>>> > > >>>> > > >>>> -- > > >>>> > > >>>> David Davis > > >>>> Software Engineer > > >>>> > > >>> > > >> > > > > > > >