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

FEAT: Add some Apache Kylin query api #54

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yqkcn
Copy link
Contributor

@yqkcn yqkcn commented Mar 16, 2023

Comment on lines 140 to 145
try:
response = self.client.post('/saved_queries', **kwargs).json()
except InternalServerError as err:
raise KylinQueryError(err)

return response
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @yqkcn, thanks for adding new API support in Kylinpy, I'll add the first quick pass review.

The KylinQueryError refers to a query that is incorrect or unreachable so a normal API shouldn't be raised for that one.

On the other hand, the KylinQueryError also be used in catching a bad query on the upstream use case.

Copy link
Contributor Author

@yqkcn yqkcn Mar 17, 2023

Choose a reason for hiding this comment

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

OK, I will add some Error Inherit from KylinError, and also add some unit tests.

@yqkcn yqkcn changed the title FEAT: Add some Apache Kylin query api WIP: FEAT: Add some Apache Kylin query api Mar 17, 2023
@yqkcn yqkcn force-pushed the yqk_add_query_api branch from f949168 to 00215c9 Compare April 7, 2023 11:19
@yqkcn yqkcn changed the title WIP: FEAT: Add some Apache Kylin query api FEAT: Add some Apache Kylin query api Apr 7, 2023
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