Feedback on PR "Add warning when inspectdb find multi-columns primary keys"

[damio]

Hi,

I got some feedback on the ticket that I should use comments to display the warning instead of stderr, since I don't really agree with that, I'm requesting more feedback to know which way I should do it.

Ticket: https://code.djangoproject.com/ticket/32234

PR: https://github.com/django/django/pull/13736

Thanks,

Damien

[Tim Graham]

Please try to make it easy to understand the issue without looking at tickets and pull requests. Even after I did that, I couldn't find why you believe it should be a warning and not a comment. Why is this situation different from all the other places Django uses comments to indicate where introspection may not be perfect?

Wouldn't a warning break the documented usage of inspectdb because there would be non-Python text in models.py from the warning output?

$ python manage.py inspectdb > models.py

[Damien]

Since the warning is to stderr, it doesn't break the main usecase of doing "python manage.py inspectdb > models.py"

Django use comment to tell that there was an error when the table introspection failed, but it should be a with a detailed traceback in my opinion.

But it looks like it's me who have an unpopular opinion so I will implement the PR with a comment instead of a warning.

--
You received this message because you are subscribed to a topic in the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/NhhpjqeQ8MQ/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/b455bb92-6b91-4f2f-bdaa-8932d3cb2893n%40googlegroups.com.

[Tim Graham]

What value does the traceback add for you? I'd think that most users don't care about the inner workings of inspectdb.

[Damien]

I agree it can be too much details but it can be useful to know which part of inspectdb failed.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/f8ce3796-dc64-470d-8ff5-ec0f400c11b4n%40googlegroups.com.