Skip to content

Example script: delete queries#121

Open
pine0619 wants to merge 1 commit into
getredash:masterfrom
pine0619:feature/add-delete-query
Open

Example script: delete queries#121
pine0619 wants to merge 1 commit into
getredash:masterfrom
pine0619:feature/add-delete-query

Conversation

@pine0619

Copy link
Copy Markdown

I added CLI script for delete(archive) query.

@restyled-io restyled-io Bot mentioned this pull request Jul 22, 2022
@pine0619 pine0619 closed this Jul 22, 2022
@pine0619 pine0619 reopened this Jul 22, 2022
@pine0619

Copy link
Copy Markdown
Author

@arikfr @susodapop

I got the following message in CI, Could you please teach me how to fix it?
#122

@susodapop

Copy link
Copy Markdown
Collaborator

Thanks for your contribution. The restyled bot enforces our code format (we use black). If a pull request didn't use black, restyled-bot automatically makes a pull request against the original pull request that adds black formatting. Since you closed your PR and reopened it, restyled bot closed its original pull request. But the check still fails because your code hasn't yet been formatted.

You can do this locally with pip install black and then black path/to/example.py. Push your change to your remote branch and the check will re-run and pass 👌

Comment thread pyproject.toml
clone-dashboard-and-queries = "redash_toolbelt.examples.clone_dashboard_and_queries:main"
export-queries = "redash_toolbelt.examples.query_export:main"
redash-migrate = "redash_toolbelt.examples.migrate:main"
delete-queries = "redash_toolbelt.examples.delete_queries:main"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this really require an example script? If it does, "delete queries" (plural) is the wrong name for it since your example accepts a single query_id (not plural).

Not every example requires an entry under tool.poetry.scripts either. The purpose of examples is to show how to use redash-toolbelt.

I suppose it's okay to have such a simple example here, but it should be named correctly.

Comment on lines +5 to +9
template = u"""/*
API KEY: {name}
*/
{query}"""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not sure the purpose of this...

Suggested change
template = u"""/*
API KEY: {name}
*/
{query}"""

)

def main(redash_url, api_key, query_id):
redash = Redash(redash_url, api_key)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You need a docstring here. Users will see it when they call

$ delete-queries --help

def main(redash_url, api_key, query_id):
redash = Redash(redash_url, api_key)
path = f'api/queries/{query_id}'
redash._delete(path)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since this is a user-facing script, you should catch exceptions explicitly to notify the user whether the operation succeeded or failed.

@susodapop susodapop changed the title Add delete query Example script: delete queries Jul 22, 2022
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