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

set feedback list task_id default to "" instead of None #131

Merged
merged 1 commit into from
Apr 1, 2024

Conversation

wenzhe-log10
Copy link
Collaborator

repro code:

from log10.feedback.feedback import Feedback

if __name__ == "__main__":
    fb = Feedback()
    fb.list()

Error due to incorrectly set task_id=None in request url and get 500 internal error.

@wenzhe-log10 wenzhe-log10 requested a review from kxtran April 1, 2024 22:27
@@ -59,7 +59,7 @@ def create(
res = self._post_request(self.feedback_create_url, json_payload)
return res

def list(self, offset: int = 0, limit: int = 50, task_id: str = None) -> httpx.Response:
def list(self, offset: int = 0, limit: int = 50, task_id: str = "") -> httpx.Response:
base_url = self._log10_config.url
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we add a check that if task_id is empty string, then we would not make an api call?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, task_id is optional and could be empty. If it's empty, it returns the existing feedback, like the feedback page in GUI.

@wenzhe-log10 wenzhe-log10 merged commit 5c2567d into main Apr 1, 2024
1 check passed
@wenzhe-log10 wenzhe-log10 deleted the wenzhe/fix_feedback_list branch April 1, 2024 23:40
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.

2 participants