Repository navigation
fix(autogenstudio): preserve DB password in alembic.ini (Fixes #7341) - #8147
Closed
maxpetrusenkoagent wants to merge 1 commit into
Closed
maxpetrusenkoagent wants to merge 1 commit into
maxpetrusenkoagent wants to merge 1 commit into
Conversation
SQLAlchemy 2.x masks passwords in str(URL) as ***, so the URL written to alembic.ini by SchemaManager._generate_alembic_ini_content() ended with a literal *** password. On first-time startup against PostgreSQL, create_all() succeeded with the real engine but Alembic generate_revision authenticated with *** and failed with 'password authentication failed', even though the same credentials worked via psql. Use URL.render_as_string(hide_password=False) so the real password is written, keeping the %% ConfigParser escaping for passwords that contain %. Adds regression tests for the plain and special-character password cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
Fixes #7341 — on first-time startup against PostgreSQL, AutoGen Studio's
Alembic migration step (
SchemaManager.generate_revision) fails withpassword authentication failedeven though the app's own engine connectssuccessfully with the same
DATABASE_URI(verified via psql).Root cause:
_generate_alembic_ini_content()serialized the engine URLwith
str(self.engine.url). In SQLAlchemy 2.x,str(URL)masks the passwordas
***(URL.__str__renders withhide_password=True). The URL written toalembic.initherefore became:Alembic's
autogeneratethen opened a fresh connection using the literalpassword
***, which PostgreSQL rejects. This only surfaces on first bootbecause
generate_revision()runs only when the database is empty; on laterrestarts the tables already exist and the migration path is skipped, which is
why the UI "works" while the startup log still shows the error.
Fix: render the URL with
URL.render_as_string(hide_password=False)so thereal password is preserved in
alembic.ini, keeping the existing%→%%escaping so passwords containing
%still survive ConfigParser interpolation.Related issue number
Closes #7341
Test plan / evidence
tests/test_schema_manager.py(RED first — bothfailed on
mainwithassert '***' == 'autogen', then passed after the fix):test_alembic_ini_preserves_database_password— the ini URL must containthe real password, never
***.test_alembic_ini_roundtrips_password_with_special_characters— a passwordwith URL-reserved characters (
p@ssw0rd) round-trips correctly throughURL rendering + ConfigParser
%%escaping.pytest tests/test_schema_manager.py tests/test_db_manager.py -q→ 5 passed; the 3 remaining errors in
test_db_manager.pyare pre-existingon clean
main(OpenAIChatCompletionClient fixture fails to construct withthe released autogen-ext on this host — unrelated to this change, verified
by running the same tests with the fix stashed).
ruff checkandruff format --checkpass on the changed files.hermes chat -Qfresh-context review of the exact diffreturned CLEAN / findings handled (see below).
(closed unmerged after ~2 months with no maintainer review). This PR adds
regression tests and an updated explanation; happy to adjust to maintainer
preference.
Checks