Re: runserver hangs forever, without log of error in console, in two cases

2021-02-09 Thread Carlton Gibson
Hi Michael. Welcome!

I think on both of these a general, "Yes please, if you want to work on it,
super!" is appropriate.

On 1, there's already a system check warning:

System check identified no issues (0 silenced).


You have 1 unapplied migration(s). Your project may not work properly until
you apply the migrations for app(s): auth.

Run 'python manage.py migrate' to apply them.

So the issue is *exactly* how do you want to beef that up? (Set up a test
case that you think SHOULD trigger the warning, then what's the minimal
change to make that happen?)

On 2, it's worth searching the Trac history for inspectdb related issues —
there's a limit to how much complexity we'd be willing to add in order to
add support beyond the basics—it's deliberately not trying to be a perfect
tool—but (again there) a proof-of-concept goes a long way there towards
acceptance (what *exactly* would a proposed change do/look like?)

Hope that helps.
If you do get into it, and get stuck on specifics, perhaps open a PR on
GitHub so that we can discuss more.

Kind Regards,

Carlton



On Mon, 8 Feb 2021 at 23:07, Michael Calve 
wrote:

> Hello, this is my first time posting, but I think bolstering django's
> error ouput system could be helpful as last week I spent a good amount of
> time debugging some issues related to django migrations
>
> 1. I began implementing DRF with DRF-Api-K
> ey and ran
> into this issue when trying to runserver without having first properly
> migrated DRF-Api-Key migrations to my SQL Server database. I forgot to add
> `rest_framework_api_key` app name to my database router so that django
> would know to migrate that app's migrations. It had been a while since I'd
> been in the db router code, and it was my mistake not to double check here
> first. However, it would be nice if django's runtime checks could have
> caught this and told me that I was implementing an app's code without first
> migrating, thus hanging the server (as opposed to having no console output
> at all), which in the beginning stages of implementing a new library,
> having no console warnings or errors makes things a bit more difficult
>
> 2. My app uses SQL Server for a backend (with django-mssql-backend driver)
> and a lot of tables are created by my team manually as opposed to with
> django and django migrations, thus not requiring a primary key or even an
> id field. On top of that, I use `inspectdb` command a ton and it is one of
> my favorite django commands/tools, if not one of my favorites of all time,
> but the problem is that inspectdb didn't add `primary_key=True` to the id
> field in the sql server table, because it wasn't declared in the table
> schema. What's really weird is that I was able to setup django and run ORM
> calls on that model in a python shell to test it before testing the DRF API
> against the new model with runserver, so I didn't think to double back and
> ensure that the id field had primary_key=True in its instantiation. I
> didn't end up figuring out the reason until 30 minutes later when I decided
> to run migrate and an error popped up in the console. It would be awesome
> if this error also popped up with runserver
>
> Had both of these issues not happened in the same day I probably wouldn't
> be bring this to your all's attention, but I think beefing up the migration
> runtime checking and adding some if its checks to runserver as well would
> be amazing!
>
> --
> 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+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/584c1978-cd39-4a02-8b16-f87251ec9f10n%40googlegroups.com
> 
> .
>

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAJwKpyRP8SOC%3D5eOeLB1wS3epXgKOfgQgp9hCwKRVapcAKvxQA%40mail.gmail.com.


Re: runserver hangs forever, without log of error in console, in two cases

2021-02-09 Thread 'Adam Johnson' via Django developers (Contributions to Django itself)
Carlton - I'm not sure the first issue is covered by the pending migrations
check. His post mentions needing to modify the DB router class for the new
app. As I understand it, Michael wants a new check that the DB router
allows migration of every app on at least one database. I'm not sure *that*
check is so feasible, since DB routers can run arbitrary code, such as
reaching out to other services, which we try to avoid in system checks. But
perhaps I'm overthinking it and such users could easily silence the check.
It'd be nice to see a first implementation.

the problem is that inspectdb didn't add `primary_key=True` to the id field
> in the sql server table, because it wasn't declared in the table schema
>

If I'm reading this correctly, you created a table without a primary key in
the DB, then used inspectdb to import it. If a field is not a primary key,
Django should not assume it is, but it could warn or error when it detects
a table has no PK.

On Tue, 9 Feb 2021 at 09:41, Carlton Gibson 
wrote:

> Hi Michael. Welcome!
>
> I think on both of these a general, "Yes please, if you want to work on
> it, super!" is appropriate.
>
> On 1, there's already a system check warning:
>
> System check identified no issues (0 silenced).
>
>
> You have 1 unapplied migration(s). Your project may not work properly
> until you apply the migrations for app(s): auth.
>
> Run 'python manage.py migrate' to apply them.
>
> So the issue is *exactly* how do you want to beef that up? (Set up a test
> case that you think SHOULD trigger the warning, then what's the minimal
> change to make that happen?)
>
> On 2, it's worth searching the Trac history for inspectdb related issues —
> there's a limit to how much complexity we'd be willing to add in order to
> add support beyond the basics—it's deliberately not trying to be a perfect
> tool—but (again there) a proof-of-concept goes a long way there towards
> acceptance (what *exactly* would a proposed change do/look like?)
>
> Hope that helps.
> If you do get into it, and get stuck on specifics, perhaps open a PR on
> GitHub so that we can discuss more.
>
> Kind Regards,
>
> Carlton
>
>
>
> On Mon, 8 Feb 2021 at 23:07, Michael Calve 
> wrote:
>
>> Hello, this is my first time posting, but I think bolstering django's
>> error ouput system could be helpful as last week I spent a good amount of
>> time debugging some issues related to django migrations
>>
>> 1. I began implementing DRF with DRF-Api-K
>> ey and
>> ran into this issue when trying to runserver without having first properly
>> migrated DRF-Api-Key migrations to my SQL Server database. I forgot to add
>> `rest_framework_api_key` app name to my database router so that django
>> would know to migrate that app's migrations. It had been a while since I'd
>> been in the db router code, and it was my mistake not to double check here
>> first. However, it would be nice if django's runtime checks could have
>> caught this and told me that I was implementing an app's code without first
>> migrating, thus hanging the server (as opposed to having no console output
>> at all), which in the beginning stages of implementing a new library,
>> having no console warnings or errors makes things a bit more difficult
>>
>> 2. My app uses SQL Server for a backend (with django-mssql-backend
>> driver) and a lot of tables are created by my team manually as opposed to
>> with django and django migrations, thus not requiring a primary key or even
>> an id field. On top of that, I use `inspectdb` command a ton and it is one
>> of my favorite django commands/tools, if not one of my favorites of all
>> time, but the problem is that inspectdb didn't add `primary_key=True` to
>> the id field in the sql server table, because it wasn't declared in the
>> table schema. What's really weird is that I was able to setup django and
>> run ORM calls on that model in a python shell to test it before testing the
>> DRF API against the new model with runserver, so I didn't think to double
>> back and ensure that the id field had primary_key=True in its
>> instantiation. I didn't end up figuring out the reason until 30 minutes
>> later when I decided to run migrate and an error popped up in the console.
>> It would be awesome if this error also popped up with runserver
>>
>> Had both of these issues not happened in the same day I probably wouldn't
>> be bring this to your all's attention, but I think beefing up the migration
>> runtime checking and adding some if its checks to runserver as well would
>> be amazing!
>>
>> --
>> 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+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-developers/584c1978-cd