Shopify / shopify-app-js

MIT License
219 stars 88 forks source link

Expose `ping` method for `@shopify/shopify-app-session-storage-redis` #724

Open kibertoad opened 3 months ago

kibertoad commented 3 months ago

Overview

Since Redis client for @shopify/shopify-app-session-storage-redis is private, there is no easy and cheap way to perform a healthcheck for the Redis connectivity of it.

It would be great if there was a ping() method exposed, which would invoke ping() on a client and return an object of this structure:

{
  isSuccessful: boolean
  error?: Error
}
kibertoad commented 3 months ago

If there is approval for this request, we can provide a PR for it

paulomarg commented 3 months ago

That sounds like a good idea to me. Another option that might also work and that would give apps more freedom would be the ability to pass in a client like:

import {createClient} from 'redis';
import {RedisSessionStorage} from '@shopify/shopify-app-session-storage-redis';

const client = createClient( ... );
const storage = new RedisSessionStorage(client, { ... });

Since we only use the dbUrl parameter to create a new client ourselves. That way you can do whatever you need to with the client without having to go through the package.

Which do you think would work better here?

kibertoad commented 3 months ago

I agree that supporting client passing would overall provide the most flexibility and is generally a viable solution for the issue at hand.

paulomarg commented 3 months ago

We'll be tracking this issue, but it might be a little bit until we can implement this. If you'd like to contribute with a PR, it'd be most welcome!

kibertoad commented 3 months ago

@paulomarg Sure, we'll pick it up. Thank you for the reply!

github-actions[bot] commented 1 month ago

We're labeling this issue as stale because there hasn't been any activity on it for 60 days. While the issue will stay open and we hope to resolve it, this helps us prioritize community requests.

You can add a comment to remove the label if it's still relevant, and we can re-evaluate it.

kibertoad commented 1 month ago

We are still planning to work on it