Ticket #29015

[askpr...@gmail.com]

Greetings !

I am looking to start contributing to the project, through ticket #29015 (https://code.djangoproject.com/ticket/29015).

How should the erroneous database-name be handled ? I am planning to abort database-creation, with an appropriate error-message on the console. Is that the correct way of doing this ? And if it is, what details should be present on the console ?

Thanks !
Priyansh Saxena

[Adam Johnson]

Hi Priyansh,

Aborting on creation with an informative message is probably wise. I'd say include as many details as you think relevant in the message, and it can be discussed more in review when there is a concrete message :)

Thanks,

Adam

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/e5ccacb4-7990-430c-8d74-82013c501514%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Adam

[askpr...@gmail.com]

Hello !

I have made a simple change to the django/db/backends/postgresql/base.py and added a NotSupportedError for the database-name. The result of the makemigrations command currently looks like this:

Traceback (most recent call last):
File "manage.py", line 15, in <module>
execute_from_command_line(sys.argv)
File "/home/priyansh/django/django/core/management/__init__.py", line 373, in execute_from_command_line
utility.execute()
File "/home/priyansh/django/django/core/management/__init__.py", line 367, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/home/priyansh/django/django/core/management/base.py", line 288, in run_from_argv
self.execute(*args, **cmd_options)
File "/home/priyansh/django/django/core/management/base.py", line 335, in execute
output = self.handle(*args, **options)
File "/home/priyansh/django/django/core/management/commands/makemigrations.py", line 92, in handle
loader.check_consistent_history(connection)
File "/home/priyansh/django/django/db/migrations/loader.py", line 274, in check_consistent_history
applied = recorder.applied_migrations()
File "/home/priyansh/django/django/db/migrations/recorder.py", line 61, in applied_migrations
if self.has_table():
File "/home/priyansh/django/django/db/migrations/recorder.py", line 44, in has_table
return self.Migration._meta.db_table in self.connection.introspection.table_names(self.connection.cursor())
File "/home/priyansh/django/django/db/backends/base/base.py", line 255, in cursor
return self._cursor()
File "/home/priyansh/django/django/db/backends/base/base.py", line 232, in _cursor
self.ensure_connection()
File "/home/priyansh/django/django/db/backends/base/base.py", line 216, in ensure_connection
self.connect()
File "/home/priyansh/django/django/db/backends/base/base.py", line 193, in connect
conn_params = self.get_connection_params()
File "/home/priyansh/django/django/db/backends/postgresql/base.py", line 155, in get_connection_params
"Database names longer than 63 characters are not supported by PostgreSQL. "
django.db.utils.NotSupportedError: Database names longer than 63 characters are not supported by PostgreSQL. Please supply a shorter NAME value in settings.DATABASES.

However, there are some issues that I would like to address here.

1. Is the NotSupportedError appropriate here ?
2. Should there be a newline after "... supported by PostgreSQL." ?
3. From the info given at http://www.postgresql.org/docs/current/interactive/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS, it is possible to modify this default limit of 63 characters. However, this is to be done by hacking the source code and recompiling PostgreSQL. How should I handle the situation when this limit is modified in the PostgreSQL source-code ?

Any advice, suggestion or comment would be really helpful :)

Thanks !
Priyansh

[Adam Johnson]

It's a lot easier to review code changes if you submit a pull request, and you'll get much more attention for a pull request with passing tests than pasting a stack trace to the mailing list. Here's the how-to: https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/ . Also I find that by going through the review checklist I can often answer my own questions.

Anyway my answers to your questions:

1. Probably

2. I don't think so, cf all the other exception messages from Django

3. Can you pull the limit from a PostgreSQL server variable? If it's not easy then don't bother supporting that case, probably 0.01% of users compile their own database server.

--
You received this message because you are subscribed to the Google Groups "Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/8640611c-2114-4e10-aebc-3bbe50b430a8%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[askpr...@gmail.com]

Hello Adam !

I created a PR as per your suggestion, and updated the ticket with the 'has patch' flag. Please review the patch, and do give your suggestions.

Thanks !
Priyansh

[askpr...@gmail.com]

Hello !

I have updated the PR as per Adam's review. However, I have not changed L155 in backends/postgresql/base.py from

if settings_dict['NAME'] is not None

to

if 'NAME' in settings_dict

None may be used to connect to the default 'postgres' db.

However, "if 'NAME' in settings_dict" prevents KeyError, but would still return True if 'NAME' is assigned a 'NoneType'.

(Referred test_default_name method in tests/backends/base/test_creation.py)

I tried testing Adam's suggestion on this, but it results in the following error:

TypeError: object of type 'NoneType' has no len()

The PR is still open and needs review.

Thanks and regards !

[Florian Apolloner]

You could use
```
if settings_dict.get('NAME') and len(settings_dict['NAME'])
```
if you wanted to prevent a KeyError too.

[Adam Johnson]

Please keep the conversation just on the pull request, and don't bug for review after a day. There are many patches to review, and half the work is done by volunteers in their spare time (I am one). Beware that there are 11,195 people subscribed to this mailing list.

--

You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/a494f2b1-3923-405b-8eed-55f75497a239%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Adam

[askpr...@gmail.com]

Hello !

Sorry for the trouble. I'll keep this in mind.

Thanks
Priyansh