Add option to disable DRF warnings about ember-django-adapter HOT 12 CLOSED

Is this really needed? I mean, warnings exist for a reason and I don't want to silence them by mistake (if of course they are meaningful and don't clutter the console)

from ember-django-adapter.

This is the only warning we have: https://github.com/dustinfarris/ember-django-adapter/blob/master/addon/adapters/drf.js#L111-L115

It's meant to be informative so that the user understands the requirements of coalesceFindRequests. My main concern is that it is not silent in production.

from ember-django-adapter.

I see, so the user should see it only during development.

We can use the ember config to allow the user to configure these. Something along these lines:

ENV['ember-django-adapter'].silence_coalesce_warning

If it sounds good, I'l open a PR during the weekend

from ember-django-adapter.

Something like that. I'm actually second-guessing this as I have been unable to find any precedent in other addons.

Separately, I've been wondering if we should convert all of our options to the style you mentioned:

ENV['ember-django-adapter'].apiHost

vs our current style:

ENV.APP.API_HOST

from ember-django-adapter.

The warning is potentially helpful for our users because the feature requires an external dependency (DRF) to be configured in a non-standard way. However, this is documented clearly so we could just remove the warning and see what happens. If we get a bunch of issues from users not understanding how to setup DRF or generally getting confused on what to do, we can always put the warning back in. I think this is my preferred solution.

If the warning is present (if it stays or we put it back in sometime in the future), it would be useful to be able to silence it so that it doesn't become useless noise when the back-end is implemented correctly.

from ember-django-adapter.

Another option would be to compare the number of requested items to the number of returned items. If the DRF filter is not set up, it will just return everything (ignoring the ids query)—so if the number of returned items is greater we know the user has not properly set up DRF and could throw the warning/error at that point. Is it worth the extra CPU cycles?

from ember-django-adapter.

@dustinfarris regarding configuration, using the format ENV['ember-cli-plugin-name'] seems to be the norm.

Changing it will brake compatibility, but being pre-1.0 I think is a good time to make such change.

As for the warnings, we can use ember's own approach. During minor versions, deprecation warnings appear. It's your decision to fix them or not and your app will work just fine, it just gonna be much harder to you to upgrade.

Here you can take the conscious decision not to modify the backend, and your app will work just fine. But if we silence or give option to silence, I'm afraid people won't get to modify the backend ever or new developers will miss that because they join the project later on.

from ember-django-adapter.

Ok, agree that the warning needs to stay in some form. @holandes22 what do you think about my suggestion above to compare the request and response length before issuing the warning?

from ember-django-adapter.

@dustinfarris Very sorry for the late response.

It is a nice idea, but as you said before, it will add an overhead to every findMany request. Maybe we can run the test during the first request, and then disable the warning? This way, we only have that overhead only one time

from ember-django-adapter.

Personally I don't think the warning should exist.

• Ember Django Adapter Default: coalesceFindRequests = false
• Django Rest Adapter Default: No custom filter exists to support coalesceFindRequests = true

The defaults work out of the box. If someone is going to change the defaults, they either know what they are doing or they are following documentation..

Basically this warning is documentation - but provided in the console..

Also, it's worth mentioning - I'm getting this warning displayed in production builds (when I run ember s -prod) - not sure if thats because I'm doing something wrong or its happening for everyone..

from ember-django-adapter.

Yeah, the warning is in production for everyone which needs to be fixed. I'll take another look at this tonight. I'm leaning toward removing it altogether.

from ember-django-adapter.

👍 for removing it.

from ember-django-adapter.