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

[4.0.0 future major release] postgresql_info: remove the db alias #801

Open
Andersson007 opened this issue Jan 28, 2025 · 14 comments
Open
Milestone

Comments

@Andersson007
Copy link
Collaborator

[4.0.0] postgresql_info: remove the db alias when in 4.0.0 release

The parameter was deprecated in #800

@Andersson007 Andersson007 added this to the 4.0.0 milestone Jan 28, 2025
@Andersson007 Andersson007 changed the title [4.0.0] postgresql_info: remove the db alias [4.0.0 future major release] postgresql_info: remove the db alias Jan 28, 2025
@toydarian
Copy link
Collaborator

There are also a few things in the user-module marked for deletion in 4.0.0. Should we remove those as well now?

@Andersson007
Copy link
Collaborator Author

There are also a few things in the user-module marked for deletion in 4.0.0. Should we remove those as well now?

@toydarian do you mean the same db parameter? if yes, what other folks think, should we replace it with login_db? cc @hunleyd

@toydarian
Copy link
Collaborator

No, I think it is mostly the priv stuff that was moved to its own module

@Andersson007
Copy link
Collaborator Author

@toydarian yes, we should remove it (as well as do other stuff in milestone 4.0.0), but not right now. we do it when releasing 4.0.0 not earlier than in late May (after next Ansible major version is released to minimize the impact of our breaking changes on users) or better even later. We haven't planned it yet.

My question is relevant about db in postgresql_user

@toydarian
Copy link
Collaborator

Ah okay, sorry, I thought that release was already ongoing.

About the db alias: I'm a fan of having things consistent, so if we do that we should do it everywhere.
This is the current state in the user-module: db=dict(type='str', default='', aliases=['login_db']),

@hunleyd
Copy link
Collaborator

hunleyd commented Jan 30, 2025

i'm all for making login_* the actual param across the collection and making the non login_* the alias (and deprecated). we have an internal rule at $dayjob to always use the login_* version and it makes the code so much more readable and self-documenting

@Andersson007
Copy link
Collaborator Author

Cool, thanks for your feedback folks, I'm also a fan of consistency.
Does anybody want to help with opening similar PRs?

@toydarian
Copy link
Collaborator

Does anybody want to help with opening #800 PRs?

I can help

@Andersson007
Copy link
Collaborator Author

@toydarian great, thanks!
just a fyi: i'm off next week, so folks don't wait for my reviews

@toydarian
Copy link
Collaborator

Do you guys think, that we do that in every module?

I see a slight difference here. For example in postgresql_user, the db/login_db is actually only used for login, as the user is independent of the database. But in postgresql_table, the db/login_db is the database where the table is actually created in. So in that case, the parameter has a different significance and calling it login_db can be misleading.

@hunleyd
Copy link
Collaborator

hunleyd commented Feb 3, 2025

But in postgresql_table, the db/login_db is the database where the table is actually created in. So in that case, the parameter has a different significance and calling it login_db can be misleading.

you can't create a table in a database you're not logged into, right? so it's still the login_db

@toydarian
Copy link
Collaborator

you can't create a table in a database you're not logged into, right? so it's still the login_db

Sure, but that is not my point. My point is, that for postgresql_user and postgresql_info, the login_db is only used for login and not changed, but in postgresql_table the database is actually changed (has a table added/removed etc), while the name login_db suggests it is only used for login.
In the postgresql_db module, we have name (alias db) for the database that is modified, but the login_db is called maintenance_db. All very confusing.

I honestly don't have a strong opinion on this, every option is fine for me.

@hunleyd
Copy link
Collaborator

hunleyd commented Feb 4, 2025

ISTM that consistency across all the modules should be a goal of the collection with the various login_* parameters referring only to the initial connectivity (who do i login as, to which db, etc) and then other parameters (as needed) for where 'the thing' is done.

But I defer to group consensus.

@keithf4
Copy link
Contributor

keithf4 commented Feb 4, 2025

ISTM that consistency across all the modules should be a goal of the collection with the various login_* parameters referring only to the initial connectivity (who do i login as, to which db, etc) and then other parameters (as needed) for where 'the thing' is done.

But I defer to group consensus.

This makes the most sense to me.

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

No branches or pull requests

4 participants