Include PDO SQLite driver (Collectives)

[Pilzinsel64]

A few apps (e.g. like Collectives and a AI app I forgot yet) requires PDO SQLite driver for some features.

Ref: https://nextcloud.github.io/collectives/administration/#searching-collectives

At the moment you either live with red error texts at the Overview page or don't use the app in question at all or you need to switch to AIO docker container.

This line seems to exclude it at the moment: https://github.com/nextcloud-snap/nextcloud-snap/blob/357ee46c68d34a04b36a421ff1ada2ece5e43a15/snap/snapcraft.yaml#L239

We already had similar requests, but I tread this different as SQLite commonly used for app data and not for productive use.

What do you think? @nextcloud-snap/developers

[pachulo]

A few apps (e.g. like Collectives and a AI app I forgot yet) requires PDO SQLite driver for some features.

Where is the documentation for those requirements?

[Pilzinsel64]

For collectives see here: https://nextcloud.github.io/collectives/administration/#searching-collectives

[scubamuc]

@Pilzinsel64 mentioned here #2787

[scubamuc]

@Pilzinsel64

A few apps (e.g. like Collectives and a AI app I forgot yet) requires PDO SQLite driver for some features....

• fulltextsearch
• memories
• collectives
• recognize
• preview-generator

[pachulo]

What's your take on this one @kyrofa ?

[scubamuc]

reference fulltextsearch #2793

[scubamuc]

reference to collectives #2787

[kyrofa]

We already had https://github.com/nextcloud-snap/nextcloud-snap/issues/2555, but I tread this different as SQLite commonly used for app data and not for productive use.

I think it's silly that these apps are using sqlite when a perfectly good database is already in use, but yeah if this is a common pattern (it seems it is) I don't see a huge problem supporting it.

[scubamuc]

@Pilzinsel64

confirm that these apps have no reference to SQLite...

• memories
• recognize
• preview-generator

tested them on #2795 and issues remain. these AI apps require node.js and or tensorflow.js and imagick-php

[Pilzinsel64]

• memories
• preview-generator

They need imagick-php. However, preview-generator works fine without, having trouble with it since years. memories I never used yet.

• recognize

Yup, that needs nodejs.

[scubamuc]

@pilzinsel, just wanted to make sure we're not chasing goblins 🎃

[pachulo]

OK, so we should be taking a decision here @nextcloud-snap/developers :

• Do we include the PHP PDO SQLite driver on the nextcloud snap so that the Searching Collectives feature of the Collectives app works?

We have to take into account that if we include it, the following will apply:

1. We will need to maintain this dependency (which shouldn't be a lot of work actually).
2. We will be opening the door for accepting other dependencies of non-core apps.
3. Am I missing something @kyrofa ?

Please, express your thoughts in the comments.

[Pilzinsel64]

In my opinion we should include it as there is no other option for those apps (like Collectives as one example) to fully support Nextcloud snap. (And speaking as a developer, it might make sense in some situations for them to use a separted SQLite database instead of adding all their stuff to the main database.)

I don't think there is a high amount of maintenance for inlcuding SQLite within the snap as it seems to be already a part of the default php build.

Not sure if there is missing anything else, but I would vote for "yes".

[kyrofa]

[...] there is no other option for those apps (like Collectives as one example) to fully support Nextcloud snap. (And speaking as a developer, it might make sense in some situations for them to use a separted SQLite database instead of adding all their stuff to the main database.)

I disagree on your latter point, which means I also disagree with your former point. That said, the fact is, several apps are using this approach, whether we agree with it or not. So we can either stand by our principles and make those apps broken in the snap, or we can acknowledge that this appears to be a trend in the app store and support that trend.

We obviously have our "we don't accept dependencies for non-core apps" principle for a reason. However, if we hold that standard so rigidly that we don't even respond to broad trends, we can end up with a super principled snap that no one uses. So as much as I wish these apps would use a different approach, I must begrudgingly agree that we should support it lest we risk a less and less useful snap. I strongly suggest we continue to hold the line on this in general, though. We are doing this not to support this app, or that app, but to support a broad trend that we're seeing across many apps.

[scubamuc]

Not sure if there is missing anything else, but I would vote for "yes"

me too, "yes"

We are doing this not to support this app, or that app, but to support a broad trend that we're seeing across many apps

agree! the snap already fully supports the default recommended apps. supporting more featured apps would certainly improve the user experience.

We obviously have our "we don't accept dependencies for non-core apps" principle for a reason. However, if we hold that standard so rigidly that we don't even respond to broad trends, we can end up with a super principled snap that no one uses

the snap has come a long way and has a great user base. snapd has made some interesting advances maybe we could build on that?

[pachulo]

I have mixed feelings about this, but I totally agree with what @kyrofa stated here:

We obviously have our "we don't accept dependencies for non-core apps" principle for a reason. However, if we hold that standard so rigidly that we don't even respond to broad trends, we can end up with a super principled snap that no one uses. So as much as I wish these apps would use a different approach, I must begrudgingly agree that we should support it lest we risk a less and less useful snap. I strongly suggest we continue to hold the line on this in general, though. We are doing this not to support this app, or that app, but to support a broad trend that we're seeing across many apps.

So is also a yes for this specific case.

[pachulo]

Builds with this enabled will be available soon on the latest/beta channel.