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

[WIP] Extract postgres_ha_admin into separate repo #174

Closed

Conversation

chessbyte
Copy link
Member

@chessbyte chessbyte commented May 23, 2017

@chessbyte chessbyte changed the title Extract postgres_ha_admin into separate repo [WIP] Extract postgres_ha_admin into separate repo May 23, 2017
@chessbyte chessbyte added the wip label May 23, 2017
@Fryguy
Copy link
Member

Fryguy commented May 23, 2017

@carbonin Please review.

@Fryguy
Copy link
Member

Fryguy commented May 23, 2017

I'm wondering if it makes sense for this to be postgres-ha_admin or pg-ha_admin (to mirror the pg-pglogical)

@carbonin
Copy link
Member

This is interesting because nothing would really "depend" on this. I'm not sure how we should handle that.

We could add it to the manageiq Gemfile, but that's not a real dependency of manageiq...

@carbonin
Copy link
Member

I'm wondering if it makes sense for this to be postgres-ha_admin or pg-ha_admin (to mirror the pg-pglogical)

It's not really as close to postgres as the other one was. This is more about repmgr and failing over db clients.

I called my first pass at gemfying this repmgr_client_failover

@chessbyte
Copy link
Member Author

@carbonin
Copy link
Member

@chessbyte Yes, but I meant a Gemfile or gemspec dependency. In other words, how will this get installed? If it is removed from manageiq-gems-pending it will not be in our bundle.

@carbonin
Copy link
Member

carbonin commented May 23, 2017

(Unless we add it to the manageiq-gems-pending gemspec) @Fryguy ?

@chessbyte
Copy link
Member Author

@carbonin manageiq-gems-pending will soon disappear. Each thing should require its own dependencies.

@carbonin
Copy link
Member

Each thing should require its own dependencies.

@chessbyte Right, which is why I raised the question originally. If this is merged, this gem will not be an explicit or implicit dependency of any other in our application. It's almost as if manageiq-appliance needs a Gemfile to require system service dependencies.

@Fryguy
Copy link
Member

Fryguy commented May 23, 2017

@carbonin I think the point is to add it to the respective gemfile where appropriate. That being said, the way I'm thinking this be better laid out is to either

  • Include this file directly in the manageiq-appliance repo, which is the only consumer

  • Extract evm_failover_monitor.rb to this new gem and rename as pg_ha_admin_failover_monitor.rb (or something) and put it in the bin directory of the gem. Then this gem would have to be included as part of the build, perhaps in the same place that we might require a future appliance_console gem. Maybe manageiq-appliance needs a Gemfile of some sort? @simaishi Thoughts?

@chessbyte chessbyte force-pushed the extract_postgres_ha_admin branch from a8b0773 to a3bb99f Compare June 7, 2017 21:46
@carbonin
Copy link
Member

carbonin commented Jun 8, 2017

When this gem is fully extracted and released, the dependency should be listed either in the file added in ManageIQ/manageiq-appliance#125 or as a dependency of the appliance_console gem as it controls stopping and starting the failover monitor.

@chessbyte chessbyte force-pushed the extract_postgres_ha_admin branch from a3bb99f to abc47fa Compare July 12, 2017 14:21
@miq-bot
Copy link
Member

miq-bot commented Jul 12, 2017

Checked commit chessbyte@abc47fa with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks fine. 🍪

@chessbyte
Copy link
Member Author

@bdunne Should I close this or can we use any of it?

@miq-bot
Copy link
Member

miq-bot commented Oct 19, 2017

This pull request is not mergeable. Please rebase and repush.

@bdunne
Copy link
Member

bdunne commented Oct 19, 2017

Closing in favor of #300

@bdunne bdunne closed this Oct 19, 2017
@chessbyte chessbyte deleted the extract_postgres_ha_admin branch May 15, 2020 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants