socotecio / django-socio-grpc-example

Simple examples of how to use DSG
Apache License 2.0
1 stars 2 forks source link

Filters in sync_example_bib_app not working - please correct #2

Closed markdoerr closed 1 year ago

markdoerr commented 1 year ago

Hi @AMontagu and @legau ,

I also created a sync version of the bibliography app and tried to add filters as it is described in the new documentation, but it is not working. I created a testing notebook to demonstrate the filters: sync_testing_notebook Could you please add the correct lines that it is working ? Thanks

markdoerr commented 1 year ago

Hi @AMontagu, I updated the sync example according to your hint in the Issue 199: sync_testing_notebook,

But throws the error: ValueError: metadata was invalid: ('FILTERS', '{"name_last": "Doe"}')

validate_metadata: INTERNAL:Illegal header key: FILTERS

Do you see, why 'FILTERS' is an illegal header key ?

AMontagu commented 1 year ago

Hello @markdoerr Yes metadata is a tuple of tuple. Look here for example: https://github.com/socotecio/django-socio-grpc/blob/master/django_socio_grpc/tests/test_filtering.py#L35

Can you create an issue to specify it in the new doc if it's missing ?

I see that this is already what you are doing. Will more investigate later on it

markdoerr commented 1 year ago

Thanks @AMontagu for looking into that - it would be nice to understand, what is going wrong there - since I think, I did everything like in the test, I am puzzled, why this error appears.

AMontagu commented 1 year ago

Hello @markdoerr

I can't launch your jupyter notebook. I am not familiar with it. When launching in the web interface i got:

---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
Cell In[1], line 3
      1 import grpc
      2 import json
----> 3 from sync_example_bib_app.grpc import sync_example_bib_app_pb2_grpc, sync_example_bib_app_pb2
      5 from datetime import datetime

ModuleNotFoundError: No module named 'sync_example_bib_app'

Do you know what I need to fo to have the relative import working ? I tried launching the jupyter notebook command in root directory, in backend_sync dir and in backend_sync/jupyter dir without success

markdoerr commented 1 year ago

@AMontagu, sorry for the notebook (I thought it is a good way, to document interacting with the code - quite common in academia and data science ;) - for your convenience, I made a simple filter client, showing the same error on my setup. To run that client, one needs to have the sync_example_bib_app package installed: in a venv install the package with poetry, as described in the repo README. You might need to run pip install -e . in the sync_example_bib_app aswell, if the "*sync_example_bib_app" is still not on the list of installed packages of pip list. Then I would start a django testserver (after setting the superuser password) and add a few Authors with some similar last names via the admin interface - I used "Doe1", "Doe2", ..., "Hugo". The simple filter client, should only display the authors with a last name starting with "Doe". I hope that helps to reproduce the error.

AMontagu commented 1 year ago

Jupyter notebook is a good way. I am just not familiar with it so don't know how to make relative import work on it. Will use the simple filter client.

To info https://github.com/socotecio/django-socio-grpc-example/blob/feature/sync_example_bibliography/README.md#run-the-gprc-client is empty

AMontagu commented 1 year ago

So @markdoerr, The issue is that grpc metadata key should be lowercase. This is a hudge mistake from the doc and the test because we, internally in our api client, make sur that this is always the case and then in the DSG code we make sure it's full uppercase. The tests are passing because we fake the all gRPC network layer and this is the one that need lowercase. Will fix it. Here is the line that does that: https://github.com/socotecio/django-socio-grpc/blob/master/django_socio_grpc/request_transformer/socio_internal_request.py#L79

Even if I found the issue there is an other issue just after in my side saying:

status = StatusCode.UNIMPLEMENTED
        details = "Method not found!"
        debug_error_string = "UNKNOWN:Error received from peer  {created_time:"2023-10-10T13:17:36.57079005+02:00", grpc_status:12, grpc_message:"Method not found!"}"

But it may come from my docker compose config or any. So I encourage you to test writing the metadata like this and tell me:

metadata = (("filters", (json.dumps(filter_as_dict))),)

I encourage you to stop working on the sync client and focus on the async one for the next example as we really want user to use the async one to avoid production really big issues.

markdoerr commented 1 year ago

Dear @AMontagu, I removed the sync branch and moved everything in the async branch, as you proposed. In there I re-created the filter example, with the new lower spelling "filter", as you suggested, but the filter client just returns all authors, not applying any filter :( (filter client still talks sync, but I will change it to async as soon as the filter is working - this should be independent of the communication mode). Do you see, why the filter is still not working ? At least there is no Error thrown. (As an example, I created 3 Authors: Doe1, Doe2, ..., "Hugo" as name_last and I want to get all authors, starting with Doe)

markdoerr commented 1 year ago

@AMontagu, I just also tested filterset_fields and there I can filter for exact matches. So the problem is the filter_class - or the right syntax to provide the filter via gRPC ?

AMontagu commented 1 year ago

@markdoerr Yeah not luck on this one for you. you mixed docs I thinks.

You made a typo. It's filterset_class and not filter_class.

https://django-filter.readthedocs.io/en/stable/guide/rest_framework.html#adding-a-filterset-with-filterset-class

I pushed correction that is working with the contains

markdoerr commented 1 year ago

@AMontagu, sorry for being blind ... now everything is working nicely. Very cool, thanks a lot !