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

Added YQ support #1

Closed
wants to merge 34 commits into from
Closed

Added YQ support #1

wants to merge 34 commits into from

Conversation

uzhastik
Copy link
Owner


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

airflow/providers/yandex/hooks/http_client.py Outdated Show resolved Hide resolved
return params

def _compose_api_url(self, path: str) -> str:
return self.config.endpoint + path
Copy link

Choose a reason for hiding this comment

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

urljoin?

Copy link
Owner Author

Choose a reason for hiding this comment

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

да ну, специально его не взял. кучу ненужного делает

return self.config.endpoint + path

def _compose_web_url(self, path: str) -> str:
return self.config.web_base_url + path
Copy link

Choose a reason for hiding this comment

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

и тут urljoin

def wait_results(self, query_id: str) -> Any:
result_set_count = self.client.wait_query_to_succeed(
query_id,
execution_timeout=timedelta(minutes=30),
Copy link

Choose a reason for hiding this comment

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

этот таймаут стоит или в аргументы конструктора вытащить или перестать задавать вообще

Copy link
Owner Author

Choose a reason for hiding this comment

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

окей, передвину в аргумент метода

self.user_agent = user_agent

# urls should not contain trailing /
self.endpoint: str = "https://api.yandex-query.cloud.yandex.net"
Copy link

Choose a reason for hiding this comment

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

стоило бы вынести в дефолтные аргументы конструктора

Copy link
Owner Author

Choose a reason for hiding this comment

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

это нужно для тестов, менять клиентам этот параметр надобности нет. только если появится YQ в KZ =))

def close(self):
self.client.close()

def create_query(self, query_text: str|None, name: str|None=None, description: str | None = None, query_type: QueryType = QueryType.ANALYTICS) -> str:
Copy link

Choose a reason for hiding this comment

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

я, кстати, вырезал тип streaming, так как подумал, что в airflow/jupyter стриминговые запросы явно лишние

Copy link
Owner Author

Choose a reason for hiding this comment

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

давай и я вырежу

self.folder_id = folder_id
self.connection_id = connection_id
self.public_ssh_key = public_ssh_key
self.service_account_id = service_account_id
Copy link

Choose a reason for hiding this comment

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

еще токен можно получить из-под текущего sa виртуальной машины, если он задан. удобно на виртуалках гонять. это http-запрос на адрес 169.xxx

self.query_id: str | None

def execute(self, context: Context) -> Any:
self.hook = YQHook(
Copy link

Choose a reason for hiding this comment

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

два execute подряд приведут к тому, что первый hook не будет остановлен

Copy link

github-actions bot commented May 1, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label May 1, 2024
@uzhastik uzhastik closed this May 6, 2024
@uzhastik uzhastik deleted the yq_support branch May 14, 2024 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants