-
Notifications
You must be signed in to change notification settings - Fork 192
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
move PGSU out to separate package #3892
move PGSU out to separate package #3892
Conversation
/update-requirements |
Starting 'update-requirements' workflow for this branch. |
cde1a78
to
361a513
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ltalirz , just some minor suggestions. I will try and have a look at psgu
itself now
361a513
to
66b0ea1
Compare
One of the main issues with `verdi quicksetup` is that it is non-trivial to figure out how to connect as the PostgreSQL superuser in a wide variety of operating systems and PostgreSQL setups. This PR factors out the code which takes care of this connection into a separate package, which can then be tested on a wide variety of setups using CI.
66b0ea1
to
ffbe6a2
Compare
/update-requirements |
Starting 'update-requirements' workflow for this branch. |
@csadorf It seems the update-requirements workflow already fails at the "git checkout" stage... see https://github.com/aiidateam/aiida-core/actions/runs/71254296 |
Codecov Report
@@ Coverage Diff @@
## develop #3892 +/- ##
=========================================
Coverage 78.00% 78.00%
=========================================
Files 457 457
Lines 33830 33708 -122
=========================================
- Hits 26390 26295 -95
+ Misses 7440 7413 -27
Continue to review full report at Codecov.
|
@sphuber This is ready for review |
I had the same issue for #3599. |
echo.echo_info('database {} already exists!'.format(dbname)) | ||
if not click.confirm('Use it (make sure it is not used by another profile)?'): | ||
dbname = click.prompt('new name', type=str, default=dbname) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize now (and not sure if this was already the case) that the code here is actually tightly coupled to the CLI. Is this code only used from the CLI, or should it be? Maybe not for this PR, but it would be good to think about this and see if we can make this code more general or really just move it to aiida.cmdline
if it really is just meant for the CLI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there are many pieces like this in the code, where certain functions can be called in "interactive" mode and then start prompting.
Once we have a clear path forward how to deal with those, we should go through the entire codebase once and move them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ltalirz . I have one general comment/question but I think that would be for another PR so I am merging this.
fixes #2837
fixes #2912
fixes #2588
fixes #3879
One of the main issues with
verdi quicksetup
is that it is non-trivialto figure out how to connect as the PostgreSQL superuser in a wide
variety of operating systems and PostgreSQL setups.
This PR factors out the code which takes care of this connection into a
separate
pgsu
package, which can then be tested on a wide variety of setupsusing CI.