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

Fix boolean cli flags #235

Merged
merged 3 commits into from
Jul 25, 2024
Merged

Fix boolean cli flags #235

merged 3 commits into from
Jul 25, 2024

Conversation

JCZuurmond
Copy link
Member

@JCZuurmond JCZuurmond commented Jul 23, 2024

Fix the boolean cli flags. Purely boolean flags are not supported in the databricks labs cli command. Therefore, we simulate flags by confirming with a y or yes.

  • Tested manually

Copy link

github-actions bot commented Jul 23, 2024

❌ 35/36 passed, 1 failed, 2 skipped, 8m47s total

❌ test_dashboards_creates_dashboard_with_replace_database: databricks.sdk.errors.platform.BadRequest: [INSUFFICIENT_PERMISSIONS] Insufficient privileges: (1.233s)
databricks.sdk.errors.platform.BadRequest: [INSUFFICIENT_PERMISSIONS] Insufficient privileges:
User does not have permission CREATE on CATALOG. SQLSTATE: 42501
08:51 DEBUG [databricks.sdk] Loaded from environment
08:51 DEBUG [databricks.sdk] Ignoring pat auth, because metadata-service is preferred
08:51 DEBUG [databricks.sdk] Ignoring basic auth, because metadata-service is preferred
08:51 DEBUG [databricks.sdk] Attempting to configure auth: metadata-service
08:51 INFO [databricks.sdk] Using Databricks Metadata Service authentication
[gw7] linux -- Python 3.10.14 /home/runner/work/lsql/lsql/.venv/bin/python
08:51 DEBUG [databricks.sdk] Loaded from environment
08:51 DEBUG [databricks.sdk] Ignoring pat auth, because metadata-service is preferred
08:51 DEBUG [databricks.sdk] Ignoring basic auth, because metadata-service is preferred
08:51 DEBUG [databricks.sdk] Attempting to configure auth: metadata-service
08:51 INFO [databricks.sdk] Using Databricks Metadata Service authentication
08:51 DEBUG [databricks.sdk] POST /api/2.0/lakeview/dashboards
> {
>   "display_name": "created_by_lsql_LFxfIR7eEOBtUrr5"
> }
< 200 OK
< {
<   "create_time": "2024-07-25T08:51:24.913Z",
<   "dashboard_id": "01ef4a6312c81bbc9447b609da64572a",
<   "display_name": "created_by_lsql_LFxfIR7eEOBtUrr5",
<   "etag": "-933759438",
<   "lifecycle_state": "ACTIVE",
<   "parent_path": "/Users/4106dc97-a963-48f0-a079-a578238959a6",
<   "path": "/Users/4106dc97-a963-48f0-a079-a578238959a6/created_by_lsql_LFxfIR7eEOBtUrr5.lvdash.json",
<   "serialized_dashboard": "{\"pages\":[{\"name\":\"fca6a42f\",\"displayName\":\"New Page\"}]}",
<   "update_time": "2024-07-25T08:51:25.066Z"
< }
08:51 DEBUG [databricks.labs.lsql.backends] [api][execute] CREATE SCHEMA hive_metastore.lsql_skawl WITH DBPROPERTIES (RemoveAfter=2024072510)
08:51 DEBUG [databricks.labs.lsql.core] Executing SQL statement: CREATE SCHEMA hive_metastore.lsql_skawl WITH DBPROPERTIES (RemoveAfter=2024072510)
08:51 DEBUG [databricks.sdk] POST /api/2.0/sql/statements/
> {
>   "format": "JSON_ARRAY",
>   "statement": "CREATE SCHEMA hive_metastore.lsql_skawl WITH DBPROPERTIES (RemoveAfter=2024072510)",
>   "warehouse_id": "TEST_DEFAULT_WAREHOUSE_ID"
> }
< 200 OK
< {
<   "statement_id": "01ef4a63-12fb-1c59-93cb-f6ac62893681",
<   "status": {
<     "error": {
<       "error_code": "BAD_REQUEST",
<       "message": "[INSUFFICIENT_PERMISSIONS] Insufficient privileges:\nUser does not have permission CREATE on CATA... (20 more bytes)"
<     },
<     "state": "FAILED"
<   }
< }
08:51 DEBUG [databricks.sdk] Loaded from environment
08:51 DEBUG [databricks.sdk] Ignoring pat auth, because metadata-service is preferred
08:51 DEBUG [databricks.sdk] Ignoring basic auth, because metadata-service is preferred
08:51 DEBUG [databricks.sdk] Attempting to configure auth: metadata-service
08:51 INFO [databricks.sdk] Using Databricks Metadata Service authentication
08:51 DEBUG [databricks.sdk] POST /api/2.0/lakeview/dashboards
> {
>   "display_name": "created_by_lsql_LFxfIR7eEOBtUrr5"
> }
< 200 OK
< {
<   "create_time": "2024-07-25T08:51:24.913Z",
<   "dashboard_id": "01ef4a6312c81bbc9447b609da64572a",
<   "display_name": "created_by_lsql_LFxfIR7eEOBtUrr5",
<   "etag": "-933759438",
<   "lifecycle_state": "ACTIVE",
<   "parent_path": "/Users/4106dc97-a963-48f0-a079-a578238959a6",
<   "path": "/Users/4106dc97-a963-48f0-a079-a578238959a6/created_by_lsql_LFxfIR7eEOBtUrr5.lvdash.json",
<   "serialized_dashboard": "{\"pages\":[{\"name\":\"fca6a42f\",\"displayName\":\"New Page\"}]}",
<   "update_time": "2024-07-25T08:51:25.066Z"
< }
08:51 DEBUG [databricks.labs.lsql.backends] [api][execute] CREATE SCHEMA hive_metastore.lsql_skawl WITH DBPROPERTIES (RemoveAfter=2024072510)
08:51 DEBUG [databricks.labs.lsql.core] Executing SQL statement: CREATE SCHEMA hive_metastore.lsql_skawl WITH DBPROPERTIES (RemoveAfter=2024072510)
08:51 DEBUG [databricks.sdk] POST /api/2.0/sql/statements/
> {
>   "format": "JSON_ARRAY",
>   "statement": "CREATE SCHEMA hive_metastore.lsql_skawl WITH DBPROPERTIES (RemoveAfter=2024072510)",
>   "warehouse_id": "TEST_DEFAULT_WAREHOUSE_ID"
> }
< 200 OK
< {
<   "statement_id": "01ef4a63-12fb-1c59-93cb-f6ac62893681",
<   "status": {
<     "error": {
<       "error_code": "BAD_REQUEST",
<       "message": "[INSUFFICIENT_PERMISSIONS] Insufficient privileges:\nUser does not have permission CREATE on CATA... (20 more bytes)"
<     },
<     "state": "FAILED"
<   }
< }
08:51 DEBUG [databricks.sdk] DELETE /api/2.0/lakeview/dashboards/01ef4a6312c81bbc9447b609da64572a
< 200 OK
< {}
[gw7] linux -- Python 3.10.14 /home/runner/work/lsql/lsql/.venv/bin/python

Running from acceptance #339

Copy link
Collaborator

@nfx nfx left a comment

Choose a reason for hiding this comment

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

lgtm

@JCZuurmond JCZuurmond enabled auto-merge (squash) July 25, 2024 08:48
@nfx nfx disabled auto-merge July 25, 2024 09:20
@nfx nfx merged commit dbfa6c1 into main Jul 25, 2024
7 of 8 checks passed
@nfx nfx deleted the fix/boolean-cli-flags branch July 25, 2024 09:20
nfx added a commit that referenced this pull request Jul 25, 2024
* Added publish flag to `Dashboards.create_dashboard` ([#233](#233)). In this release, we have added a `publish` flag to the `Dashboards.create_dashboard` method, allowing users to publish the dashboard upon creation, thereby resolving issue [#219](#219). This flag is included in the `labs.yml` file with a description of its functionality. Additionally, the `no-open` flag's description has been updated to specify that it prevents the dashboard from opening in the browser after creation. The `create_dashboard` function in the `cli.py` and `dashboards.py` files has been updated to include the new `publish` flag, allowing for more flexibility in how users create and manage their dashboards. The `Dashboards.create_dashboard` method now calls the `WorkspaceClient.lakeview.publish` method when the `publish` flag is set to `True`, which publishes the created dashboard. This behavior is covered in the updated tests for the method.
* Fixed boolean cli flags ([#235](#235)). In this release, we have improved the handling of command-line interface (CLI) flags in the `databricks labs` command. Specifically, we have addressed the limitation that pure boolean flags are not supported. Now, when using boolean flags, the user will be prompted to confirm with a `y` or 'yes'. We have modified the `create_dashboard` command to accept string inputs for the `publish` and `no_open` flags, which are then converted to boolean values for internal use. Additionally, we have introduced a new `open-browser` command, which will open the dashboard in the browser after creating when set to `y` or 'yes'. These changes have been tested manually to ensure correct behavior. This improvement provides a more flexible input experience and better handling of boolean flags in the CLI command for software engineers using the open-source library.
* Fixed format breaks widget ([#238](#238)). In this release, we've made significant changes to the 'databricks/labs/lsql' directory's 'dashboards.py' file to address formatting breaks in the widget that could occur with Call to Action (CTA) presence in a query. These changes include the addition of new class variables, including _SQL_DIALECT and _DIALECT, and their integration into existing methods such as _parse_header, validate, format, _get_abstract_syntax_tree, and replace_catalog_and_database_in_query. Furthermore, we have developed new methods for creating and deleting schemas and getting the current test purge time. We have also implemented new integration tests to demonstrate the fix for the formatting issue and added new test cases for the query handler's header-splitting functionality, query formatting, and CTE handling. These enhancements improve the library's handling of SQL queries and query tiles in the context of dashboard creation, ensuring proper parsing, formatting, and metadata extraction for a wide range of query scenarios.
* Fixed replace database when catalog or database is None ([#237](#237)). In this release, we have addressed an issue where system tables disappeared in ucx dashboards when replacing the placeholder database. To rectify this, we have developed a new method, `replace_catalog_and_database_in_query`, in the `dashboards.py` file's `replace_database` function. This method checks if the catalog or database in a query match the ones to be replaced and replaces them with new ones, ensuring that system tables are not lost during the replacement process. Additionally, we have introduced new unit tests in `test_dashboards.py` to verify that queries are correctly transformed when replacing the database or catalog in the query. These tests include various scenarios, using two parametrized test functions, to ensure the correct functioning of the feature. This change provides a more robust and reliable dashboard display when replacing the placeholder database in the system.
@nfx nfx mentioned this pull request Jul 25, 2024
nfx added a commit that referenced this pull request Jul 25, 2024
* Added publish flag to `Dashboards.create_dashboard`
([#233](#233)). In this
release, we have added a `publish` flag to the
`Dashboards.create_dashboard` method, allowing users to publish the
dashboard upon creation, thereby resolving issue
[#219](#219). This flag is
included in the `labs.yml` file with a description of its functionality.
Additionally, the `no-open` flag's description has been updated to
specify that it prevents the dashboard from opening in the browser after
creation. The `create_dashboard` function in the `cli.py` and
`dashboards.py` files has been updated to include the new `publish`
flag, allowing for more flexibility in how users create and manage their
dashboards. The `Dashboards.create_dashboard` method now calls the
`WorkspaceClient.lakeview.publish` method when the `publish` flag is set
to `True`, which publishes the created dashboard. This behavior is
covered in the updated tests for the method.
* Fixed boolean cli flags
([#235](#235)). In this
release, we have improved the handling of command-line interface (CLI)
flags in the `databricks labs` command. Specifically, we have addressed
the limitation that pure boolean flags are not supported. Now, when
using boolean flags, the user will be prompted to confirm with a `y` or
'yes'. We have modified the `create_dashboard` command to accept string
inputs for the `publish` and `no_open` flags, which are then converted
to boolean values for internal use. Additionally, we have introduced a
new `open-browser` command, which will open the dashboard in the browser
after creating when set to `y` or 'yes'. These changes have been tested
manually to ensure correct behavior. This improvement provides a more
flexible input experience and better handling of boolean flags in the
CLI command for software engineers using the open-source library.
* Fixed format breaks widget
([#238](#238)). In this
release, we've made significant changes to the 'databricks/labs/lsql'
directory's 'dashboards.py' file to address formatting breaks in the
widget that could occur with Call to Action (CTA) presence in a query.
These changes include the addition of new class variables, including
_SQL_DIALECT and _DIALECT, and their integration into existing methods
such as _parse_header, validate, format, _get_abstract_syntax_tree, and
replace_catalog_and_database_in_query. Furthermore, we have developed
new methods for creating and deleting schemas and getting the current
test purge time. We have also implemented new integration tests to
demonstrate the fix for the formatting issue and added new test cases
for the query handler's header-splitting functionality, query
formatting, and CTE handling. These enhancements improve the library's
handling of SQL queries and query tiles in the context of dashboard
creation, ensuring proper parsing, formatting, and metadata extraction
for a wide range of query scenarios.
* Fixed replace database when catalog or database is None
([#237](#237)). In this
release, we have addressed an issue where system tables disappeared in
ucx dashboards when replacing the placeholder database. To rectify this,
we have developed a new method, `replace_catalog_and_database_in_query`,
in the `dashboards.py` file's `replace_database` function. This method
checks if the catalog or database in a query match the ones to be
replaced and replaces them with new ones, ensuring that system tables
are not lost during the replacement process. Additionally, we have
introduced new unit tests in `test_dashboards.py` to verify that queries
are correctly transformed when replacing the database or catalog in the
query. These tests include various scenarios, using two parametrized
test functions, to ensure the correct functioning of the feature. This
change provides a more robust and reliable dashboard display when
replacing the placeholder database in the system.
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