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

Release notes foreign keys #683

Merged
merged 3 commits into from Mar 31, 2020
Merged

Release notes foreign keys #683

merged 3 commits into from Mar 31, 2020

Conversation

@PVince81
Copy link
Member

PVince81 commented Mar 4, 2019

@patrickjahns you have suggested mentioning the addition of foreign keys in the release notes.

See my draft. I'm not sure what else to add there, did you have anything in mind ?
Something like "if your database / setup doesn't support it, you should ... do what ?"

@PVince81 PVince81 added this to the Development milestone Mar 4, 2019
@PVince81 PVince81 self-assigned this Mar 4, 2019
@PVince81

This comment has been minimized.

Copy link
Member Author

PVince81 commented Mar 4, 2019

cc @pmaier1 FYI

This topic came up when some people had troubles with the foreign keys during update.
So far the causes aren't known yet.

See:

@patrickjahns

This comment has been minimized.

Copy link
Member

patrickjahns commented Mar 5, 2019

Something like "if your database / setup doesn't support it, you should ... do what ?"

Hmm - there are a couple of things to check ( like having the right storage engine for mysql/mariadb (innodb)) - but other than that - we kind of require this currently and no fallback in place

Other ideas @DeepDiver1975 ?

Copy link
Contributor

settermjd left a comment

While it's good that users are notified, via this update, that foreign keys are utilised, because of the WebDAV locks feature; just telling them that isn't enough. As others have said, adding more comprehensive information about them, issues that may be encountered, and how to correct those issues should also be added.

@settermjd

This comment has been minimized.

Copy link
Contributor

settermjd commented Mar 6, 2019

After reading through the linked issues, the debug information and resolution steps should definitely be added to an FAQ/help section.

Copy link
Contributor

settermjd left a comment

As covered in the comments, this, imho, needs further work.

@phil-davis

This comment has been minimized.

Copy link
Contributor

phil-davis commented Mar 6, 2019

Yes, for me when I read just the fews lines here so far, I think "well isn't that interesting, the software uses a database feature that has existed for decades." But I don't think "oh, maybe my database won't support or work with this for some reason", because in my decades in the industry I have never contemplated a production installation of a relational database where foreign keys do not work.

@pmaier1

This comment has been minimized.

Copy link
Contributor

pmaier1 commented Mar 14, 2019

For whom is this information relevant and why?
As @settermjd points out, we need to provide these information as otherwise there's no value of adding it here - it rather causes confusion, IMO.

@settermjd

This comment has been minimized.

Copy link
Contributor

settermjd commented Mar 18, 2019

Can you comment further, @PVince81.

@PVince81

This comment has been minimized.

Copy link
Member Author

PVince81 commented Mar 18, 2019

This is not ready for merging. Need someone with more deployment knowledge to chime in and explain what an admin should be aware of.

Or should we just say something like "it's the first time we introduce foreign keys so there is a slight chance that you'll experience issues during upgrades if you have an exotic setup" ? @patrickjahns

@settermjd

This comment has been minimized.

Copy link
Contributor

settermjd commented Jun 25, 2019

@settermjd

This comment has been minimized.

Copy link
Contributor

settermjd commented Oct 25, 2019

@PVince81, as Patrick Jahns has moved on, who is the best person to talk to about #683?

modules/admin_manual/pages/release_notes.adoc Outdated Show resolved Hide resolved
modules/admin_manual/pages/release_notes.adoc Outdated Show resolved Hide resolved
modules/admin_manual/pages/release_notes.adoc Outdated Show resolved Hide resolved
modules/admin_manual/pages/release_notes.adoc Outdated Show resolved Hide resolved
@mmattel

This comment has been minimized.

Copy link
Contributor

mmattel commented Mar 31, 2020

remind readers that SQLite is not recommended/supported for production use anyway

👍

@phil-davis

This comment has been minimized.

Copy link
Contributor

phil-davis commented Mar 31, 2020

The number of commits has got crazy here - I will rebase and squash
Done

@phil-davis phil-davis force-pushed the rn-foreign-keys branch from 902f6d0 to 0f152a1 Mar 31, 2020
This moves SQLite down the database order, so that it, hopefully,
doesn't seem as prominent nor important. It also adds an admonition
reaffirming that SQLite is not supported in production deployments.
Copy link
Contributor

phil-davis left a comment

LGTM - can be backported as far as you want.
Since it is and addition to a quite old part of the release notes, do we backport further than usual? Up to you!

@settermjd

This comment has been minimized.

Copy link
Contributor

settermjd commented Mar 31, 2020

LGTM - can be backported as far as you want.
Since it is and addition to a quite old part of the release notes, do we backport further than usual? Up to you!

Current policy is to only backport to the latest two release branches.

@settermjd settermjd merged commit e12dbbe into master Mar 31, 2020
1 check passed
1 check passed
continuous-integration/drone/pr Build is passing
Details
@delete-merged-branch delete-merged-branch bot deleted the rn-foreign-keys branch Mar 31, 2020
settermjd added a commit that referenced this pull request Mar 31, 2020
Backport of PR #683
settermjd added a commit that referenced this pull request Mar 31, 2020
Backport of PR #683
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.