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

move feature from %sqlrender -> %sqlcmd snippets #647

Closed
edublancas opened this issue Jun 22, 2023 · 3 comments · Fixed by #681
Closed

move feature from %sqlrender -> %sqlcmd snippets #647

edublancas opened this issue Jun 22, 2023 · 3 comments · Fixed by #681
Assignees

Comments

@edublancas
Copy link

we're going to deprecate %sqlrender, we should move the logic to %sqlcmd snippets

currently, this is how it works:

%%sql --save penguins
select * from penguins.csv limit 10
%%sql --save subset
select * from penguins where island is not null
cte = %sqlrender subset
print(cte)

prints:

WITH penguins AS (
select * from penguins.csv limit 10
)
select * from penguins where island is not null

Now, with the change, we should get the same when executing: %sqlcmd snippets subset

i.e.,

cte = %sqlcmd snippets subset
print(cte)

should print:

WITH penguins AS (
select * from penguins.csv limit 10
)
select * from penguins where island is not null
@bbeat2782
Copy link

bbeat2782 commented Jun 23, 2023

Acceptance Criteria:

  1. Make %sqlcmd snippets {name} gives the same output as %sqlrender {name}
  2. Add FutureWarning to %sqlrender {name} that includes a link to %sqlcmd snippets documentation
  3. Update %sqlcmd snippets documentation under API Reference tab to include explanation and behavior of %sqlcmd snippets {name}
  4. Modify test functions for %sqlrender {name} to test %sqlcmd snippets {name}

@edublancas
Copy link
Author

Add FutureWarning to %sqlrender {name} that includes a link to %sqlcmd snippets documentation

should also include the equivalent command. e.g., if user ran %sqlrender stuff, then the warning should tell the user to migrate to %sqlcmd snippets stuff

@bbeat2782
Copy link

Okay. I will include that in the error message that the FutureWarning shows.

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 a pull request may close this issue.

2 participants