Add PostgreSQL version restriction #16171
Conversation
| def initialize(*args) | ||
| super | ||
| if postgresql_version < 90400 || postgresql_version >= 90600 | ||
| raise "The version of PostgreSQL being connected to is incompatible with ManageIQ." |
There was a problem hiding this comment.
Do we need to productize this?
There was a problem hiding this comment.
I was wondering the same thing 😬 Is there a mechanism (i18n, whatever) that we use for such things?
There was a problem hiding this comment.
I18n.t("product.name") I think should do it.
There was a problem hiding this comment.
If it was just in the logs I think we wouldn't need to but because it's a raise it could get to the UI I suppose?
There was a problem hiding this comment.
I agree. I think the idea was to be an in-your-face error and not just a warning. I'll add that for sure.
ManageIQ is only compatible with PostgreSQL 9.4 or 9.5, and many odd errors take a while to find when it's a simple version problem that could be found immediately. This adds a restriction to the AR adapter to maintain lazy-loading the connection as expected.
chrisarcand
force-pushed
the
fail-unless-pg-95
branch
from
276bac1
to
8f5cf8c
Compare
|
Checked commit chrisarcand@8f5cf8c with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 config/initializers/postgres_required_versions.rb
|
|
This is problematic :(. Maybe not so much for our users, but for developers, definitely. PG9.5 is simply too old, Debian stable offers 9.6 for comparison. And while there's a bunch of issues with 9.6 or 10.0, the simple fact is, it works. (And getting 9.5 installed is non-trivial on some systems already.) So... can we relax the check a bit? For example, by limiting it to |
|
...and since our users use the appliance, which is presumably fine... what was the point of this? I don't apparently know many people actually using 9.5... so this just broke miq for all of us. @chrisarcand Can you please be specific about the actual problem you were trying to fix? (And unless we want to force new developers to compile their DB from scratch, 9.6 needs to be supported Real Soon Now.) EDIT: also, if this does become a thing, please update https://github.com/ManageIQ/guides/blob/master/developer_setup.md with instructions on how to install 9.5 on current fedoras, macs and debian :), since this makes the guide lie :). |
|
Fedora 26 offers pg 9.6 😞 |
|
@himdel The issue being solved is the one mentioned in the PR description 😄 When users configure the appliance to point to an "external" database, say one on bare metal or some other database cluster not running on our appliance, we need to ensure that things will work as planned. Debian may ship with a newer version of PG, but I think the PG package in RHEL updates repo is still 9.2. This is the case we are protecting against. I'm for hiding this behind a production env flag. For developers, I would suggest installing postgresql from the official repos which are versioned by major release (i.e. 9.5 is a separate repo from 9.6). This should give you more of a choice around which version to install and will even allow you to install multiple versions at once 😄 See https://www.postgresql.org/download/ for details. |
|
@carbonin aah, issue description, thanks :). So, this is about external DB. Ok, that makes sense.. but definitely not for devel setups :). So.. tentatively creating PR to limit this to |
If someone does, I'm fine with giving a work around as needed. I don't think we should try to account for this case in code. |
|
I agree, throw this behind a production conditional.
If they are insistent on using production mode for development, they should also be insistent on using an actual supported version of Postgres or what's the point? I'm PTO until Wednesday, so PR and merge away. |
|
Created #16197, thanks :) |
@himdel By the way, setting up a running Docker container with any version of Postgres and changing your database.yml to use it takes no time at all. I'm PTO now but do ask in the core Gitter channel and I'm sure someone can assist you if you need it. |
|
@chrisarcand Thanks!, but that's not really the issue. If you have a classroom full of students with 9.6 preinstalled on school systems (which we can't reinstall), and the students are really new to git (so can't afford to keep local changes), there is no good option :). And if you're already using newer PG features in your other DBs and don't want to keep another instance running just for miq, and especially with docker where you also have to manually make it start (and stop) there is no good option either. I did not know about https://www.postgresql.org/download, that helps a lot if needed, but I think I'll just keep using PG10.0, presumably things will only get better, not worse :). (That said, maybe we should update the devel guide to actively prefer https://www.postgresql.org/download over distro default for postgres install.) |
I don't agree. Each extra step rises the bar for contribution. |
Ahh I understand your challenges with using Docker there, sure.
I agree that things like Docker raise the bar for contribution. But simply put, we just don't support anything above PostgreSQL 9.5 yet and you're encouraged to use a supported version (one that's actually used on running appliances) using whatever method (Docker, local, etc) is best for you. Using 9.6 or 10 is at your own risk - which is totally great for those that consciously accept that, and why I 100% agree with the change just made to restrict it only in production. My only point here is that a bug found with PG 10 won't really be treated with the same sort of urgency, and it's generally recommended you use the same versions of things that are actually used on appliances, in production. :) |
|
I use this: https://github.com/chargio/postgresql-vagrant And I don't have to worry about versions, it is always the same in all systems (linux, mac, any versions of those). Includes postgresql 9.5 and memcached. But you have to change the configuration as you can't use sockets, and thus you need to define host: localhost in database.yml |
Fixes #16145
ManageIQ is only compatible with PostgreSQL 9.4 or 9.5, and many odd errors take a while to find when it's a simple version problem that could be found immediately.
This adds a restriction to the AR adapter to maintain lazy-loading the connection as expected.
Tests are knowingly omitted because any such test really doesn't tell us anything in the future as stubbing the underlying adapter methods would render it moot and actually bothering setting up the adapter being loaded/unloaded isn't worth the trouble given the previous point.