Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure string column limit of 4.000 characters #31679

Merged

Conversation

@nickvergessen nickvergessen added the 3. to review Waiting for reviews label Mar 23, 2022
@nickvergessen nickvergessen added this to the Nextcloud 24 milestone Mar 23, 2022
@blizzz
Copy link
Member

blizzz commented Mar 23, 2022

Column "oc_external_config"."value" is type String, but exceeding the 4.000 length limit.

PHP Fatal error:
Uncaught Error: Class 'OCA\Files_External\Tests\Command\CommandTest' not found in
/drone/src/apps/files_external/tests/Command/ApplicableTest.php:31

also

Column "oc_ldap_user_mapping"."ldap_dn" is type String, but exceeding the 4.000 length limit.

@nickvergessen
Copy link
Member Author

🍿

@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
@nickvergessen nickvergessen force-pushed the bugfix/noid/ensure-string-columns-to-be-maximum-of-4000 branch from ce43174 to 9d2536e Compare March 25, 2022 11:44
@blizzz blizzz mentioned this pull request Mar 31, 2022
@come-nc come-nc merged commit dd357d7 into master Mar 31, 2022
@come-nc come-nc deleted the bugfix/noid/ensure-string-columns-to-be-maximum-of-4000 branch March 31, 2022 09:37
@tcitworld
Copy link
Member

List of apps that are concerned that I could find:

@marcelklehr
Copy link
Member

This breaking change should be listed in #29914

Also, it would be nice if apps could opt out of supporting Oracle. There have been multiple "Oracle cannot do X" issues lately, which are really annoying to work around, IMHO.

@nickvergessen
Copy link
Member Author

nickvergessen commented Apr 3, 2022

Also, it would be nice if apps could opt out of supporting Oracle.

You can already by specifying the supported databases, e.g.
https://github.com/nextcloud/twofactor_webauthn/blob/27977e5ffef14f2d280628ff51c4728e300d8a1d/appinfo/info.xml#L20-L22
But we really recommend to instead aim for supporting all databases

This breaking change should be listed in #29914

We can list it of course, but it is not new in theory. It errored before already on Oracle. If you install the app with a github action on Oracle it should show imediately.
See https://github.com/nextcloud/app-tutorial/blob/master/.github/workflows/phpunit-oci.yml for example code

@marcelklehr
Copy link
Member

marcelklehr commented Apr 3, 2022

It errored before already on Oracle.

I didn't even know I had to support Oracle, until these issues crept up.

<database>sqlite</database>
<database>mysql</database>
<database>pgsql</database>

Is a nice fix, didn't know this was possible. Thanks :)

@marcelklehr
Copy link
Member

I still get this error, though, even when excluding oracle as a dependency.

https://github.com/nextcloud/bookmarks/runs/5807151749

@skjnldsv
Copy link
Member

skjnldsv commented Apr 7, 2022

I still get this error, though, even when excluding oracle as a dependency.
nextcloud/bookmarks/runs/5807151749

@marcelklehr
#31872

@marcelklehr
Copy link
Member

My issue was fixed by #31825 already :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants