You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "sejal-gupta-ksolves (via GitHub)" <gi...@apache.org> on 2026/06/23 18:49:24 UTC
[PR] cli: Fix TypeError in 'airflow db shell' when database name is missing [airflow]
sejal-gupta-ksolves opened a new pull request, #68913:
URL: https://github.com/apache/airflow/pull/68913
closes: #68897
### Problem
When configuring the `AIRFLOW__DATABASE__SQL_ALCHEMY_CONN` environment variable without explicitly appending a database name component to the URI path (e.g., `postgresql://postgres:postgres@postgres:5432`), the command `airflow db check` passes gracefully. However, running `airflow db shell` crashes with an unhandled, cryptic `TypeError: expected str, bytes or os.PathLike object, not NoneType`.
This occurs because the underlying URL parser extracts the missing database component as `None`, which is then forwarded to a subprocess environment dictionary mapping (`env["PGDATABASE"] = url.database`), ultimately crashing internal operating system utility calls (`os.fsencode`).
### Solution
Updated the `shell` command function inside `airflow/cli/commands/db_command.py` to add a global validation guardrail check right after retrieving the database URL structure. If `url.database` evaluates to `None` or is empty, the command now proactively raises a clean, standard Python built-in `ValueError` detailing that the configuration is invalid.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] cli: Fix TypeError in 'airflow db shell' when database name is missing [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #68913:
URL: https://github.com/apache/airflow/pull/68913#issuecomment-5129500484
### Backport successfully created: v3-3-test
Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>✅</td>
<td>v3-3-test</td>
<td><a href="https://github.com/apache/airflow/pull/70752"><img src="https://img.shields.io/badge/PR-70752-blue" alt="PR Link"></a></td>
</tr>
</table>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] cli: Fix TypeError in 'airflow db shell' when database name is missing [airflow]
Posted by "phanikumv (via GitHub)" <gi...@apache.org>.
phanikumv commented on code in PR #68913:
URL: https://github.com/apache/airflow/pull/68913#discussion_r3466191716
##########
airflow-core/tests/unit/cli/commands/test_db_command.py:
##########
@@ -534,6 +534,15 @@ def test_cli_shell_invalid_ppg3(self):
with pytest.raises(AirflowException, match=r"Unknown driver: invalid\+psycopg"):
db_command.shell(self.parser.parse_args(["db", "shell"]))
+ @mock.patch(
+ "airflow.cli.commands.db_command.settings.engine.url",
+ make_url("postgresql+psycopg2://postgres:airflow@postgres/"),
+ )
+ def test_db_shell_missing_database_name(self):
+ """Assert that an explicit ValueError is raised when the database name is missing."""
+ with pytest.raises(ValueError, match="The metadata database name is missing"):
+ db_command.shell(self.parser.parse_args(["db", "shell"]))
Review Comment:
```suggestion
@pytest.mark.parametrize(
"conn_uri",
[
pytest.param("postgresql://postgres:postgres@postgres:5432", id="db-name-none"),
pytest.param("postgresql://postgres:postgres@postgres:5432/", id="db-name-empty"),
],
)
def test_db_shell_missing_database_name(self, conn_uri):
"""Assert that an explicit ValueError is raised when the database name is missing."""
with mock.patch("airflow.cli.commands.db_command.settings.engine.url", make_url(conn_uri)):
with pytest.raises(ValueError, match="The metadata database name is missing"):
db_command.shell(self.parser.parse_args(["db", "shell"]))
```
##########
airflow-core/tests/unit/cli/commands/test_db_command.py:
##########
@@ -534,6 +534,15 @@ def test_cli_shell_invalid_ppg3(self):
with pytest.raises(AirflowException, match=r"Unknown driver: invalid\+psycopg"):
db_command.shell(self.parser.parse_args(["db", "shell"]))
+ @mock.patch(
+ "airflow.cli.commands.db_command.settings.engine.url",
+ make_url("postgresql+psycopg2://postgres:airflow@postgres/"),
+ )
+ def test_db_shell_missing_database_name(self):
+ """Assert that an explicit ValueError is raised when the database name is missing."""
+ with pytest.raises(ValueError, match="The metadata database name is missing"):
+ db_command.shell(self.parser.parse_args(["db", "shell"]))
Review Comment:
```suggestion
@pytest.mark.parametrize(
"conn_uri",
[
pytest.param("postgresql://postgres:postgres@postgres:5432", id="db-name-none"),
pytest.param("postgresql://postgres:postgres@postgres:5432/", id="db-name-empty"),
],
)
def test_db_shell_missing_database_name(self, conn_uri):
"""Assert that an explicit ValueError is raised when the database name is missing."""
with mock.patch("airflow.cli.commands.db_command.settings.engine.url", make_url(conn_uri)):
with pytest.raises(ValueError, match="The metadata database name is missing"):
db_command.shell(self.parser.parse_args(["db", "shell"]))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] cli: Fix TypeError in 'airflow db shell' when database name is missing [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #68913:
URL: https://github.com/apache/airflow/pull/68913#discussion_r3681846284
##########
airflow-core/tests/unit/cli/commands/test_db_command.py:
##########
@@ -534,6 +534,23 @@ def test_cli_shell_invalid_ppg3(self):
with pytest.raises(AirflowException, match=r"Unknown driver: invalid\+psycopg"):
db_command.shell(self.parser.parse_args(["db", "shell"]))
+ @mock.patch(
Review Comment:
This decorator patches `settings.engine.url` to a fixed `postgresql+psycopg2://postgres:airflow@postgres/`, but the first thing the test body does is patch the same target again with the parametrized `conn_uri` — so the decorator's value is never the one under test.
It's harmless (the second positional argument is `new`, so no mock is injected into the signature, which is why the parameter list still works), but it's dead and misleading — a reader has to work out which patch wins. Dropping the decorator and keeping just the `with mock.patch(...)` inside says exactly what the test means.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] cli: Fix TypeError in 'airflow db shell' when database name is missing [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #68913:
URL: https://github.com/apache/airflow/pull/68913
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] cli: Fix TypeError in 'airflow db shell' when database name is missing [airflow]
Posted by "phanikumv (via GitHub)" <gi...@apache.org>.
phanikumv commented on code in PR #68913:
URL: https://github.com/apache/airflow/pull/68913#discussion_r3466203578
##########
airflow-core/tests/unit/cli/commands/test_db_command.py:
##########
@@ -534,6 +534,15 @@ def test_cli_shell_invalid_ppg3(self):
with pytest.raises(AirflowException, match=r"Unknown driver: invalid\+psycopg"):
db_command.shell(self.parser.parse_args(["db", "shell"]))
+ @mock.patch(
+ "airflow.cli.commands.db_command.settings.engine.url",
+ make_url("postgresql+psycopg2://postgres:airflow@postgres/"),
+ )
+ def test_db_shell_missing_database_name(self):
+ """Assert that an explicit ValueError is raised when the database name is missing."""
+ with pytest.raises(ValueError, match="The metadata database name is missing"):
+ db_command.shell(self.parser.parse_args(["db", "shell"]))
+
Review Comment:
The URL the bug was actually reported with is `postgresql://postgres:postgres@postgres:5432` — no trailing slash, which parses to `url.database = None`. That `None` is what blew up the original code
(`env["PGDATABASE"] = None`).
Your test uses the trailing-slash form `postgresql+psycopg2://postgres:airflow@postgres/`, which gives you `''` instead of `None`. The test isn't feeding in the exact input from the report.
Easiest fix is to just cover both.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] cli: Fix TypeError in 'airflow db shell' when database name is missing [airflow]
Posted by "phanikumv (via GitHub)" <gi...@apache.org>.
phanikumv commented on code in PR #68913:
URL: https://github.com/apache/airflow/pull/68913#discussion_r3466200019
##########
airflow-core/tests/unit/cli/commands/test_db_command.py:
##########
@@ -534,6 +534,15 @@ def test_cli_shell_invalid_ppg3(self):
with pytest.raises(AirflowException, match=r"Unknown driver: invalid\+psycopg"):
db_command.shell(self.parser.parse_args(["db", "shell"]))
+ @mock.patch(
+ "airflow.cli.commands.db_command.settings.engine.url",
+ make_url("postgresql+psycopg2://postgres:airflow@postgres/"),
+ )
+ def test_db_shell_missing_database_name(self):
+ """Assert that an explicit ValueError is raised when the database name is missing."""
+ with pytest.raises(ValueError, match="The metadata database name is missing"):
+ db_command.shell(self.parser.parse_args(["db", "shell"]))
+
Review Comment:
```suggestion
@pytest.mark.parametrize(
"conn_uri",
[
pytest.param("postgresql://postgres:postgres@postgres:5432", id="db-name-none"),
pytest.param("postgresql://postgres:postgres@postgres:5432/", id="db-name-empty"),
],
)
def test_db_shell_missing_database_name(self, conn_uri):
"""Assert that an explicit ValueError is raised when the database name is missing."""
with mock.patch("airflow.cli.commands.db_command.settings.engine.url", make_url(conn_uri)):
with pytest.raises(ValueError, match="The metadata database name is missing"):
db_command.shell(self.parser.parse_args(["db", "shell"]))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org