-
Notifications
You must be signed in to change notification settings - Fork 33
[IDEV-2271] RE:RTUF stream responses: address PR feedbacks #181
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
Conversation
domaintools/results.py
Outdated
| self.latest_feeds_status_code = response.status_code | ||
| # assigned the status code to `latest_feeds_status_code` | ||
| # started the process | ||
| yield {"status_ready": True} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to yield and the self.latest_feeds_status_code line above. You can instead just call self.setStatus() here instead of in line :252
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. Thanks!
| # yield the rest of the feeds | ||
| yield from feeds_generator | ||
| except Exception as e: | ||
| log.error(f"FATAL: Failed to start the feed generator in _make_request. Reason: {e}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to just raise an Exception for the user you don't need to layer the try: except here and in the other functions. You can just remove all the try: except:'s and let the exception happen with the error text.
For example if the user has a bad value with after=abc here is what the error will look like:
(venv) (IDEV-2271)~/github.com/python_api$ python tester.py
Traceback (most recent call last):
File "/home/kdealca/github.com/python_api/domaintools/results.py", line 252, in _get_results
self.setStatus(self.latest_feeds_status_code) # set the status already
File "/home/kdealca/github.com/python_api/domaintools/base_results.py", line 201, in setStatus
raise BadRequestException(code, reason)
domaintools.exceptions.BadRequestException: None
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/kdealca/github.com/python_api/domaintools/results.py", line 270, in response
yield from feed_response_generator
File "/home/kdealca/github.com/python_api/domaintools/results.py", line 257, in _get_results
self.setStatus(500, reason_text=e)
File "/home/kdealca/github.com/python_api/domaintools/base_results.py", line 207, in setStatus
raise InternalServerErrorException(code, reason)
domaintools.exceptions.InternalServerErrorException: None
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/kdealca/github.com/python_api/tester.py", line 9, in <module>
for r in results.response():
File "/home/kdealca/github.com/python_api/domaintools/results.py", line 279, in response
self.setStatus(500, reason_text=e)
File "/home/kdealca/github.com/python_api/domaintools/base_results.py", line 207, in setStatus
raise InternalServerErrorException(code, reason)
domaintools.exceptions.InternalServerErrorException: None
(venv) (IDEV-2271)~/github.com/python_api$
It is 3 exceptions deep which makes it difficult to understand. And the helpful text from the server:
(IDEV-2271)~/github.com/python_api$ curl -H "X-API-Key: $DT_API_KEY" "https://api.domaintools.com/v1/feed/nod?after=abc"
{"error":"Bad request: invalid value in after query parameter; see documentation for examples"}
(IDEV-2271)~/github.com/python_api$
Is nowhere to be seen.
Instead you can do something like this:
def _make_request(self) -> Generator:
"""
Creates and manages the httpx stream request, yielding data line by line.
This is the core generator that communicates with the DT frontend API server.
"""
session_info = self._get_session_params_and_headers()
headers = session_info.get("headers")
parameters = session_info.get("parameters")
with httpx.stream(
"GET",
self.url,
headers=headers,
params=parameters,
verify=self.api.verify_ssl,
proxy=self.api.proxy_url,
timeout=None,
) as r:
error_text = ""
if r.status_code != 200 or r.status_code != 206:
r.read()
error_text = r.text
self.setStatus(r.status_code, reason_text=error_text)
for line in r.iter_lines():
yield line
def data(self) -> Generator:
self._data = self._get_results()
return self._data
def response(self) -> Generator:
while self.status != 200:
yield from self.data()
if not self.kwargs.get("sessionID"):
# we'll only do iterative request for queries that has sessionID. Otherwise, we will have an infinite
# request if sessionID was not provided but the required data asked is more than the maximum (1 hour of data)
break
self._status = None
If there is an error there will only be one, and any message from the server will be shown:
(venv) (IDEV-2271)~/github.com/python_api$ python tester.py
Traceback (most recent call last):
File "/home/kdealca/github.com/python_api/tester.py", line 9, in <module>
for r in results.response():
File "/home/kdealca/github.com/python_api/domaintools/results.py", line 248, in response
yield from self.data()
File "/home/kdealca/github.com/python_api/domaintools/results.py", line 237, in _make_request
self.setStatus(r.status_code, reason_text=error_text)
File "/home/kdealca/github.com/python_api/domaintools/base_results.py", line 201, in setStatus
raise BadRequestException(code, reason)
domaintools.exceptions.BadRequestException: {"error":"Bad request: invalid value in after query parameter; see documentation for examples"}
(venv) (IDEV-2271)~/github.com/python_api$
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the feedback @kelvinatorr! will do the corresponding changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
domaintools/results.py
Outdated
| for line in response.iter_lines(): | ||
| yield line | ||
|
|
||
| def _get_results(self) -> Generator: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can now get rid of _get_results() completely and use _make_request() directly in data()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done. thanks!
domaintools/results.py
Outdated
| import json | ||
| import logging | ||
| import httpx | ||
| import time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
time is now not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed.
kelvinatorr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing my concerns
Changes: