njlg / perl-rethinkdb

A Pure Perl RethinkDB Driver
http://njlg.info/perl-rethinkdb
Artistic License 2.0
22 stars 5 forks source link

$package::_rdb_io #29

Open ufobat opened 8 years ago

ufobat commented 8 years ago

Hello,

I am not perfectly sure what you want to do but i am pretty sure that this line is wrong:

https://github.com/njlg/perl-rethinkdb/blob/master/lib/Rethinkdb.pm#L34

it uses the variable $_rdb_io in the package "package". $package is not used lexically.

ufobat commented 8 years ago

and there you write to the package "package". maybe you can use https://metacpan.org/source/NJLG/Rethinkdb-0.13/lib/Rethinkdb/IO.pm#L90

package Rethinkdb {
    our $_rdb_io;
}
ufobat commented 8 years ago

Hi again,

this now is more a wish then a bug report. I wanted to reuse my connection, and i am hot sure if the IO::Socket::INET maybe looses the connection somehow. what do you think of this?


sub rethink_workaround {
    my $self = shift;
    # we know that rehink does some package pollution and captures
    # the connection into $package::_rdb_io

    if ($package::_rdb_io) {
        # there is a connection
        my $socket = $package::_rdb_io->{_handle};
        if ($socket->connected) {
        }else {
            # reconnection
            $package::_rdb_io->reconnect();
        }
    } else {
        # there is no connection
        # create a new one and store it in $package::_rdb_io
        r->connect->repl;
    }

    die 'there is no rethink connection available' unless ($package::_rdb_io);
}

# in the main code
# rethink_workaround() instead of r->connect->repl()
rethink_workaround();
r->whatever();
njlg commented 8 years ago

I am not sure what your first two comments mean. You don't have to use the repl syntax to "store" the connection, but if you do we have to cache that some where... and that somewhere is in the caller. (e.g. my $package = caller;)

You don't have to use the repl functionality. You can create a connection and pass it into the run function.

Example:

my $io = r->connect;
my $res = r->db('test')->tables->run($io);
ufobat commented 8 years ago

the thing is that my $package = caller(); $package::foo, is not using $foo in the package of the caller but in the package 'package'. i mean literally 'package'.

 % perl -wle 'package what {our $foo = 1;} package main { sub do_something {my $what = caller; print "package: $what"; print "package global variable is: $what::foo"; }; do_something()};'
package: main
package global variable is: 1

vs

 % perl -wle 'package what {our $foo = 1;} package main { sub do_something {my $different = caller; print "package: $different"; print "package global variable is: $different::foo"; }; do_something()};'
Name "different::foo" used only once: possible typo at -e line 1.
package: main
Use of uninitialized value $different::foo in concatenation (.) or string at -e line 1.
package global variable is: 
njlg commented 8 years ago

Ah! I see what you are saying. I'll try to make a fix for this. Is there any bug with the driver doing this? I should probably add a test case for it.

ufobat commented 8 years ago

No I dont think so. Maybe documentation upon this.

Personally, I just dislike the idea of storing this into the package of the caller because it's the callers "home". I would recommend to store the connection into your own package namespace, as i said in my first comment.

I am also going to rewrite my workaround to store the $io manually. Only one further question:is there a way within the rethinkdb driver to check or reconnect if the underlying connection in $io->{_handle} disconnected/timeouted or something?

njlg commented 8 years ago

You can do either of these to reconnect:

r->io->reconnect;

# or if you have the connection saved:
my $io = r->connect;
$io->reconnect;
ufobat commented 8 years ago

your code example always reconnects. I am looking for a way to reconnect on demand. If i am going to keep my own Rethinkdb::IO and I just want to reconnect for example if the connection got lost or timed out. Somethink like that would be cool.

 my $socket = $package::_rdb_io->{_handle};
        if ($socket->connected) {
        }else {
            # reconnection
            $package::_rdb_io->reconnect();
        }
njlg commented 8 years ago

It is difficult to determine how/when/why the connection failed for all use cases. For now, I just leave it up to the callees/users to decide when it is appropriate to reestablish a connection.