Skip to content
This repository has been archived by the owner on Nov 8, 2018. It is now read-only.

Increase Address column width to 256 #1433

Merged
merged 1 commit into from
Apr 21, 2016

Conversation

ChristophWurst
Copy link
Contributor

fixes #1421

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @DeepDiver1975, @LukasReschke and @jancborchardt to be potential reviewers

@ChristophWurst
Copy link
Contributor Author

@otbe @tahaalibra @Scheirle easy to review :-)

@jancborchardt
Copy link
Contributor

@otbe @tahaalibra since you reported / were involved in the original issue, can you please review this?

@@ -25,6 +25,8 @@

namespace OCA\Mail\Controller;

use Exception;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we are using this...we can change \Exception $ex to Exception $ex on line 236

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed. Thanks

@ChristophWurst ChristophWurst force-pushed the addresscollector-address-column-width branch 2 times, most recently from 7d0b3d9 to 8e5a06a Compare April 20, 2016 19:14
@ChristophWurst ChristophWurst force-pushed the addresscollector-address-column-width branch from 8e5a06a to ce190d5 Compare April 20, 2016 19:51
@ChristophWurst ChristophWurst merged commit 4d4ad3d into master Apr 21, 2016
@ChristophWurst ChristophWurst deleted the addresscollector-address-column-width branch April 21, 2016 19:15
@viki53
Copy link

viki53 commented May 10, 2016

Are you sure 256 is a good idea? MySQL doesn't like VARCHAR to be over a size of 255… And the automatic update script shows it:

Doctrine\DBAL\Exception\DriverException: An exception occurred while executing 'ALTER TABLE oc_mail_collected_addresses CHANGE `email` `email` VARCHAR(256) NOT NULL': SQLSTATE[42000]: Syntax error or access violation: 1071 Specified key was too long; max key length is 767 bytes

Looks like you pushed it into production without enough testing… ;)

And now I can't access my server because the update failed and still keeps failing ⚠️ 🔥

@ChristophWurst
Copy link
Contributor Author

Hi @viki53,
you're right, we don't have enough people to test this app with all possible database configurations. You are very welcome to help us test released. Let me know and I'll ping you before the next release! Thanks.

Btw. this has been fixed already. Keep in mind this app is still under development ;-)

@viki53
Copy link

viki53 commented May 10, 2016

Okay, so when will this fix be pushed to the OC servers? Because in the meantime, my server is kinda dead, as the update fails to pass and the server stays in maintenance mode. :-/

I'd be glad to test some apps, but I don't really use this one enough to test it (I just have it in case I'd need a webmail for some of my addresses some day).

@ChristophWurst
Copy link
Contributor Author

This is an open source project, it's done when it's done ;-)

It depends on which owncloud core version you're using. On newer ones, manually patching will make the code integrity check fail. So that might not be an option.
Apart from that, you could simply apply the fix (edit the database.xml file) and do the update again. I'm not totally sure if that works, but you could give it a try.

@viki53
Copy link

viki53 commented May 10, 2016

I did try fixing that manually, but I still have the old file (with 64) in place, not the new one.

The update triggers the error I mentioned earlier, so I guess it's not applied and the new files not saved.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants