postgres-plugin / tags-system

A reusable tags system
4 stars 0 forks source link

Closing the client #25

Closed jay-meister closed 7 years ago

jay-meister commented 7 years ago

Currently I am creating a connection in our plugin:

var pg = require('pg');
var client = new pg.Client();

// connect the client
client.connect(function (err) {
  Hoek.assert(!err, err);
});

However the tests hang as there is no way to close the client. I tried to expose a closing function in the plugin:

  server.app.closeClient = function () {
    // disconnect the client
    client.end(function (err) {
      Hoek.assert(!err, err);
    });
  };

And run the function in the test file:

tape('teardown', function (t) {
  server.app.closeClient();
  t.end();
});

However it gives me the following failure:

screen shot 2016-11-07 at 11 42 50
jay-meister commented 7 years ago

No matter where I run that client.end function, it gives me the same error.

jay-meister commented 7 years ago

I am happy to look into it a bit more. But would appreciate a quick look over @SimonLab in case you have seen this before?

jay-meister commented 7 years ago

Going to pick my way through https://github.com/dwyl/hapi-postgres-connection/blob/master/index.js for a solution

jay-meister commented 7 years ago

I'm going to try going down the pg.connect route

SimonLab commented 7 years ago

I had this error with redis and the way I resolve this is by making sure to have a reference to my redis object that I can close (a bit like you've done with the closeClient function). I think I used https://github.com/dwyl/redis-connection to do this. I think it's a good idea to try to see how hapi-postres-connection is resolving this issue.

jay-meister commented 7 years ago

That is exactly what I have tried to do. But it seems to be an error in my overall implementation of creating the client then closing it.

It seems that this pg.connect might be the way to go so am investigating that now.

jay-meister commented 7 years ago

It seems like I will be re-writing the hapi-postgres-connection plugin if I am to implement this properly.

Do we need to create a client in the plugin?

SimonLab commented 7 years ago

Ok maybe it might be worth to try to just pass the client to the plugin as an option (see #4 ). Hopefully you will be able to run your tests and we can still try to search if there is another solution

jay-meister commented 7 years ago

So should I add hapi-postgres-connection to our project?

SimonLab commented 7 years ago

I didn't have a proper look yet at hapi-postgres-connection but if it make the application easier to use with Postgres, yes we can try. Have you try to just use a simple client and to pass it to the plugin?

jay-meister commented 7 years ago

I have not. But my problem is closing the client so the tests don't hang. I worry that if i create a client and pass it to the plugin, I will still have the same problem. I think hapi-postgres-connection does that work for us.

jay-meister commented 7 years ago

Will attempt to implement hapi-postgres-connection into the app

jay-meister commented 7 years ago

Annoyingly I am still having problems when using Hapi-postgres-connection.

The problem being, I cannot access the client that hpc creates in a plugin, because the hpc plugin hasn't had enough time to initialise itself yet. This is more or less the same issue I was having before with trying to attach a done function to server.app. By the time the tests were run, the function hadn't been added to server.app.

Possible solutions:

I never knew that registering plugins could be such a pain.