Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of warning #27

Closed
dbrgn opened this issue Feb 8, 2019 · 3 comments
Closed

Get rid of warning #27

dbrgn opened this issue Feb 8, 2019 · 3 comments
Labels

Comments

@dbrgn
Copy link
Owner

dbrgn commented Feb 8, 2019

I have a use case where I need to dynamically determine the nested serializer like this:

    def get_config(self, tracker: Tracker):
        config = tracker.get_config()
        if config is not None:
            serializer = TRACKER_CONFIG_SERIALIZERS.get(tracker.model)
            if serializer is None:
                logger.warning('Could not find serializer for model %s', tracker.model)
                return None
            return serializer(config).data
        return None

This works but results in a warning, because the nested serializer does not have access to the request in the context.

If we pass in the context...

            return serializer(config, context=self.context).data

...then the warning is gone, but the filtering is applied to those fields too.

The problem is that drf-dynamic-fields cannot know that this serializer is a nested serializer because it's used like a root serializer. And I'm afraid of playing some tricks with parent/child attributes, since that can go wrong easily.

I see two approaches to solve this:

  • Remove the warning (no filtering if there's no request in the context)
  • Add a special field to the context (e.g. dynamic_fields_ignore) that is queried in order to ignore the filtering on this serializer.

I'm undecided. What do you think @jtrain?

@dbrgn dbrgn added the bug label Feb 8, 2019
@jtrain
Copy link
Collaborator

jtrain commented Feb 8, 2019

i don't have any special love for the warning. Removing it would be a positive thing for my code.

As for optionally warning or not, there might be a few ways to go about it.

  1. As you said we could have some special field set on the serializer somewhere; or
    2 it could be a settings.py setting; or
  2. We keep the warning, but document how you'd turn it off with the python logging framework, though that would mean turning all warnings for the lib off, not just this one; or
  3. we demote the warning to a log level or debug level warning so people could switch them off

@jtrain
Copy link
Collaborator

jtrain commented Feb 8, 2019

Finally another option completely (to work around this with existing code) is to provide a clean serializer in your codebase without the dynamic field mixin, and a second serializer which simply inherits the first with the mixin. e.g.

SchoolSerializer; and
DynamicSchoolSerializer

@dbrgn
Copy link
Owner Author

dbrgn commented Mar 1, 2019

As you said we could have some special field set on the serializer somewhere; or
2 it could be a settings.py setting

I like that. Implemented in #28.

@dbrgn dbrgn closed this as completed in #28 Mar 1, 2019
dbrgn added a commit that referenced this issue Mar 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants