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

Make conn_id optional in SQL files when using aql.render() #117

Closed
tatiana opened this issue Feb 18, 2022 · 0 comments · Fixed by #148
Closed

Make conn_id optional in SQL files when using aql.render() #117

tatiana opened this issue Feb 18, 2022 · 0 comments · Fixed by #148
Assignees
Labels
bug Something isn't working priority/high High priority
Milestone

Comments

@tatiana
Copy link
Collaborator

tatiana commented Feb 18, 2022

Context

At the moment, conn_id needs to be specified in all SQL files used by aql.render(); otherwise, Astro (0.5.1) raises the following exception:

airflow.exceptions.AirflowNotFoundException: The conn_id `None` isn't defined

The current Astro documentation is misleading since it does not include this, e.g.:
https://github.com/astro-projects/astro#setting-up-sql-files

# join_customers_and_orders.sql
SELECT c.customer_id, c.source, c.region, c.member_since,
        CASE WHEN purchase_count IS NULL THEN 0 ELSE 1 END AS recent_purchase
        FROM orders c LEFT OUTER JOIN customers p ON c.customer_id = p.customer_id

Desired behavior

Users should specify a default conn_id to apply to all files in aql.render(), and overwrite in individual files if needed. This would be consistent with specifying the conn_id` in the other operators.

Acceptance criteria

  • Have a way of specifying a default conn_id when using aql.render.
  • SQL files used by aql.render do not need to declare conn_id
  • All the documentation and examples continue working as expected.
  • There is documentation covering this feature.
  • Ask @mag3141592 (bug reporter) to review the PR
@tatiana tatiana added the bug Something isn't working label Feb 18, 2022
@tatiana tatiana added this to the 0.6.1 milestone Feb 21, 2022
@tatiana tatiana added the priority/high High priority label Feb 21, 2022
@dimberman dimberman self-assigned this Mar 7, 2022
@dimberman dimberman linked a pull request Mar 7, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority/high High priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants