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

fix: DataCollector sql_queries model not found on filter(request=self… #476

Merged
merged 1 commit into from
Sep 3, 2021

Conversation

armanjtehrani
Copy link
Contributor

Closes #475

After digging into silk DataCollector, I figured out in the second line of this section of the code no sql query is returned. So the part where adds model to query dictionaries, can't do it's job and we face the error issued in #475

models.SQLQuery.objects.bulk_create(sql_queries)
sql_queries = models.SQLQuery.objects.filter(request=self.request)

By changing these 2 lines to sql_queries = models.SQLQuery.objects.bulk_create(sql_queries), sql_queries is no longer empty and the rest of the system can function correctly.

@codecov
Copy link

codecov bot commented Jun 13, 2021

Codecov Report

Merging #476 (5b90a3a) into master (7e93d6c) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #476      +/-   ##
==========================================
- Coverage   82.83%   82.82%   -0.01%     
==========================================
  Files          50       50              
  Lines        2051     2050       -1     
==========================================
- Hits         1699     1698       -1     
  Misses        352      352              
Impacted Files Coverage Δ
silk/collector.py 79.41% <100.00%> (-0.16%) ⬇️
silk/__init__.py
silk/urls.py
project/project/__init__.py 60.00% <0.00%> (ø)
project/project/urls.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e93d6c...5b90a3a. Read the comment docs.

Copy link
Contributor

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please add a test for the changes?

@armanjtehrani
Copy link
Contributor Author

can you please add a test for the changes?

I currently don't know exactly for what reason this issue happens. Should check it out further.

@Archmonger
Copy link
Contributor

Fairly insignificant code coverage change, and I don't believe tests are needed for just changing how the variable is assigned.

Will merge.

@Archmonger Archmonger merged commit 44f6755 into jazzband:master Sep 3, 2021
Archmonger added a commit that referenced this pull request Sep 3, 2021
This reverts commit 44f6755, reversing
changes made to 6fb58b8.
@Archmonger
Copy link
Contributor

Archmonger commented Sep 3, 2021

Reverted this merge due to this issue with meta profiling

Exception when performing meta profiling, dumping trace below
Traceback (most recent call last):
  File "C:\Users\username\Documents\Repositories\Conreq\.venv\lib\site-packages\silk\middleware.py", line 132, in _process_response
    collector.finalise()
  File "C:\Users\username\Documents\Repositories\Conreq\.venv\lib\site-packages\silk\collector.py", line 189, in finalise
  File "C:\Users\username\Documents\Repositories\Conreq\.venv\lib\site-packages\django\db\models\fields\related_descriptors.py", line 1015, in set
    self.add(*new_objs, through_defaults=through_defaults)
  File "C:\Users\username\Documents\Repositories\Conreq\.venv\lib\site-packages\django\db\models\fields\related_descriptors.py", line 950, in add
    self._add_items(
  File "C:\Users\username\Documents\Repositories\Conreq\.venv\lib\site-packages\django\db\models\fields\related_descriptors.py", line 1126, in _add_items
    target_ids = self._get_target_ids(target_field_name, objs)
  File "C:\Users\username\Documents\Repositories\Conreq\.venv\lib\site-packages\django\db\models\fields\related_descriptors.py", line 1062, in _get_target_ids
    raise ValueError(
ValueError: Cannot add "<SQLQuery: SQLQuery object (None)>": the value for field "sqlquery" is None

@auvipy
Copy link
Contributor

auvipy commented Sep 6, 2021

you should not have merge this on the first place.

@Archmonger
Copy link
Contributor

I agree with you, my mistake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when using profiler
3 participants