You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2023/01/05 22:29:59 UTC

[GitHub] [superset] diegomedina248 opened a new pull request, #22611: dm/ch42084/migrate superset queries last updated ms to api v1

diegomedina248 opened a new pull request, #22611:
URL: https://github.com/apache/superset/pull/22611

   - build(deps): bump ws and @types/ws in /superset-websocket (#22327)
   - fix: Styling fixes for horizontal filter bar (#22337)
   - chore: Refactor python libs (#22335)
   - fix: Missing spacing in adhoc filters popover (#22346)
   - chore: Bump holidays to 0.17.2 (#22354)
   - fix: Broken effect in useCSSTextTruncation hook (#22324)
   - feat(explore-popover): Show disabled 'Save' button in explore popover (#21318)
   - fix(bigquery): Properly display errors for BigQuery DBs (#22349)
   - refactor(frontend): Make dashboard search box the first filter (#19721)
   - fix: Reordering native filters ignored by filter bar (#22362)
   - chore(docs): adding community calendar link to the community page (#22347)
   - fix: Change dropdown in Alert/Report modal to use javascript for conditional rendering instead of css (#22360)
   - fix: Time filter position and click in Horizontal FilterBar (#22338)
   - fix: DropdownContainer items width calculation (#22371)
   - fix: Add tooltip to dropdown trigger in horizontal filter bar (#22373)
   - fix: Button resizing in horizontal filter bar (#22365)
   - fix(snowflake): Allow encrypted_extra field to be imported (#22357)
   - feat(dashboard): Add edit button to dashboard native filters filter cards (#22364)
   - chore(viz): rename v1 and v2 charts (#22369)
   - fix(translations): French translation (typo fixes) (#21942)
   - fix(readme): link target and title (#19576)
   - fix: make sure that gsheets db connection form loads properly (#22361)
   - chore(plugin-chart-echarts): upgrade to echarts 5.4.1 (#22382)
   - fix: Update typo in docker-add-drivers.mdx (#21965)
   - feat: Add oneLine mode to AsyncSelect (#22379)
   - perf: Prevent rerendering and re-querying metadata of filters in horizontal bar (#22389)
   - fix: correct exception level in log and add error message (#22381)
   - fix: make database connection modal ace fields uncontrolled (#22350)
   - build(deps-dev): bump @types/node from 18.11.10 to 18.11.13 in /superset-websocket (#22386)
   - fix(report): Capture unexpected errors in report screenshots. Fixes #21653 (#21724)
   - build(deps-dev): bump eslint from 7.32.0 to 8.29.0 in /superset-websocket (#22322)
   - build(deps): bump express from 4.17.1 to 4.18.2 in /docs (#22341)
   - chore(deps): bump css-what from 2.1.2 to 2.1.3 in /superset-frontend (#21712)
   - chore: show database UUID in API (#22411)
   - build(deps): bump loader-utils from 2.0.2 to 2.0.4 in /docs (#22134)
   - fix(sqla): copy temporal range logic to helper (#22405)
   - feat(thumbnails): add support for user specific thumbs (#22328)
   - fix(trino): Fix Trino timestamp conversion (#21737)
   - fix: Force configuration for SafeMarkdown component in Handlebars (#22417)
   - fix: Allow empty CSS in Handlebars (#22422)
   - fix(dashboard): Update owners of dashboard list after editing (#22383)
   - test: Fix act errors in VizTypeControl test (#22424)
   - fix(hive): Fix regression from #21943 (#22431)
   - chore: set Snowflake user agent (#22432)
   - build(deps): bump qs from 6.5.2 to 6.5.3 in /superset-frontend (#22343)
   - fix(chart-table): Scrollbar causing header + footer overflow (#21064)
   - fix: remove unsupported REST API search col with dotted notation on c… (#22440)
   - build(deps-dev): bump typescript from 4.2.3 to 4.9.4 in /superset-websocket (#22414)
   - build(deps): bump uuid and @types/uuid in /superset-websocket (#22412)
   - chore(deps): bump express from 4.18.1 to 4.18.2 in /superset-websocket/utils/client-ws-app (#21754)
   - chore: Re-add inheritance of Presto macros for Trino et al. (#22435)
   - fix(cypress): Fix failing/flaky E2E tests (#22460)
   - fix: fix comment in Docker environment files (#22421)
   - chore(viz): Rename legacy non-time-series Bar Chart (#22430)
   - fix: Create dataset polish/bug fix (#22262)
   - build(deps): bump @ant-design/icons from 4.2.2 to 4.8.0 in /superset-frontend (#22158)
   - fix: Fixed spacing in alert modal (#22066)
   - chore: adding additional code owners for cypress tests (#22476)
   - fix(chart-list): Hide 'Dashboards added to' column. (#22475)
   - feat(welcome): make examples tab customizable (#22302)
   - fix(assets api): import replaces dashboard (#22208)
   - fix(cypress): disable flaky tests (#22512)
   - build(deps): bump qs from 6.5.2 to 6.5.3 in /superset-frontend/cypress-base (#22340)
   - chore: updating changelog and updating (#22479)
   - fix(init): Initialize _jwt_cookie_name  in AsyncQueryManager __init__  (#22314)
   - chore(deps-dev): bump @typescript-eslint/parser from 5.45.0 to 5.47.0 in /superset-websocket (#22465)
   - build(deps): bump pip-compile-multi from 2.4.1 to 2.6.1 in /requirements (#22216)
   - build(deps): bump loader-utils from 1.4.0 to 1.4.2 in /superset-embedded-sdk (#22142)
   - chore: Update dataset_id & dataset_type datasource_id & datasource_type for SPA explore (#22461)
   - chore: Bump Pillow to 9.3.0 (#22489)
   - feat: update time comparison choices (again) (#22458)
   - refactor: rename filter_rel_fields to base_related_field_filters (#22508)
   - feat(trino): support early cancellation of queries (#22498)
   - chore(ssh-tunnel): Refactor establishing raw connection with contextmanger (#22366)
   - fix(explore): datasource_type typo (#22543)
   - build(deps): bump fast-json-patch from 3.1.0 to 3.1.1 in /docs (#22557)
   - build(deps): bump wheel from 0.37.0 to 0.38.1 in /requirements (#22533)
   - fix: adding missing examples for bubble chart, bullet chart, calendar heatmap chart and country map chart in the gallery (#22523)
   - fix(localization): pybabel doesn't extract plural forms from frontend  (#22507)
   - build(deps): bump json5 from 2.2.1 to 2.2.2 in /docs (#22561)
   - fix(websocket): bump ts-node to fix startup error (#22563)
   - fix(cypress): make test chart time range deterministic (#22567)
   - fix(explore): support saving undefined time grain (#22565)
   - feat(ssh-tunnelling):  Setup SSH Tunneling Commands for Database Connections  (#21912)
   - chore: Use visibilityToggle prop to control password input visibility (#22363)
   - chore: upgrade interweave (#22572)
   - chore: upgrade react-ace (#22573)
   - chore: upgrade react-checkbox-tree (#22583)
   - fix(helm): Fixing up chart and linting (#22590)
   - chore: adding missing examples in the gallery for several chart types (#22597)
   - chore: Migrate /superset/queries/<last_updated_ms> to API v1
   
   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] diegomedina248 commented on a diff in pull request #22611: chore: Migrate /superset/queries/ to API v1

Posted by GitBox <gi...@apache.org>.
diegomedina248 commented on code in PR #22611:
URL: https://github.com/apache/superset/pull/22611#discussion_r1063753749


##########
superset/queries/api.py:
##########
@@ -123,3 +142,56 @@ class QueryRestApi(BaseSupersetModelRestApi):
     base_related_field_filters = {"database": [["id", DatabaseFilter, lambda: []]]}
     allowed_rel_fields = {"database", "user"}
     allowed_distinct_fields = {"status"}
+
+    @expose("/updated_since")
+    @protect()
+    @safe
+    @rison(queries_get_updated_since_schema)
+    @statsd_metrics
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
+        f".get_updated_since",
+        log_to_statsd=False,
+    )
+    def get_updated_since(self, **kwargs: Any) -> FlaskResponse:
+        """Get a list of queries that changed after last_updated_ms
+        ---
+        get:
+          summary: Get a list of queries that changed after last_updated_ms
+          parameters:
+          - in: query
+            name: q
+            content:
+              application/json:
+                schema:
+                  $ref: '#/components/schemas/queries_get_updated_since_schema'
+          responses:
+            200:
+              description: Queries list
+              content:
+                application/json:
+                  schema:
+                    type: object
+                    properties:
+                      result:
+                        description: >-
+                          A List of queries that changed after last_updated_ms
+                        type: array
+                        items:
+                          $ref: '#/components/schemas/{{self.__class__.__name__}}.get'
+            400:
+              $ref: '#/components/responses/400'
+            401:
+              $ref: '#/components/responses/401'
+            404:
+              $ref: '#/components/responses/404'
+            500:
+              $ref: '#/components/responses/500'
+        """
+        try:
+            last_updated_ms = kwargs["rison"].get("last_updated_ms", 0)
+            queries = QueryDAO.get_queries_changed_after(last_updated_ms)
+            payload = [q.to_dict() for q in queries]

Review Comment:
   The problem here is that this model is different than the way we're currently using schemas, the FE expects more params than the QuerySchema, most of them in camelCase. There're other apis, like the sql_json that do the same.
   Should we make the change now for this or wait til the v1 transition is done to move ahead with that?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] EugeneTorap commented on pull request #22611: chore: Migrate /superset/queries/ to API v1

Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #22611:
URL: https://github.com/apache/superset/pull/22611#issuecomment-1397629952

   @dpgaspar Merge it?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] dpgaspar commented on a diff in pull request #22611: chore: Migrate /superset/queries/ to API v1

Posted by GitBox <gi...@apache.org>.
dpgaspar commented on code in PR #22611:
URL: https://github.com/apache/superset/pull/22611#discussion_r1063421017


##########
superset/queries/api.py:
##########
@@ -123,3 +142,56 @@ class QueryRestApi(BaseSupersetModelRestApi):
     base_related_field_filters = {"database": [["id", DatabaseFilter, lambda: []]]}
     allowed_rel_fields = {"database", "user"}
     allowed_distinct_fields = {"status"}
+
+    @expose("/updated_since")
+    @protect()
+    @safe
+    @rison(queries_get_updated_since_schema)
+    @statsd_metrics
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
+        f".get_updated_since",
+        log_to_statsd=False,
+    )
+    def get_updated_since(self, **kwargs: Any) -> FlaskResponse:
+        """Get a list of queries that changed after last_updated_ms
+        ---
+        get:
+          summary: Get a list of queries that changed after last_updated_ms
+          parameters:
+          - in: query
+            name: q
+            content:
+              application/json:
+                schema:
+                  $ref: '#/components/schemas/queries_get_updated_since_schema'
+          responses:
+            200:
+              description: Queries list
+              content:
+                application/json:
+                  schema:
+                    type: object
+                    properties:
+                      result:
+                        description: >-
+                          A List of queries that changed after last_updated_ms
+                        type: array
+                        items:
+                          $ref: '#/components/schemas/{{self.__class__.__name__}}.get'
+            400:
+              $ref: '#/components/responses/400'
+            401:
+              $ref: '#/components/responses/401'
+            404:
+              $ref: '#/components/responses/404'
+            500:
+              $ref: '#/components/responses/500'
+        """
+        try:
+            last_updated_ms = kwargs["rison"].get("last_updated_ms", 0)
+            queries = QueryDAO.get_queries_changed_after(last_updated_ms)
+            payload = [q.to_dict() for q in queries]

Review Comment:
   can you replace this with a marshmallow schema dump instead?



##########
tests/integration_tests/queries/api_tests.py:
##########
@@ -392,3 +393,58 @@ def test_get_list_query_no_data_access(self):
         # rollback changes
         db.session.delete(query)
         db.session.commit()
+
+    def test_get_updated_since(self):
+        """
+        Query API: Test get queries updated since timestamp
+        """
+        now = datetime.utcnow()
+        client_id = self.get_random_string()
+
+        admin = self.get_user("admin")
+        example_db = get_example_database()
+
+        old_query = self.insert_query(
+            example_db.id,
+            admin.id,
+            self.get_random_string(),
+            sql="SELECT col1, col2 from table1",
+            select_sql="SELECT col1, col2 from table1",
+            executed_sql="SELECT col1, col2 from table1 LIMIT 100",
+            changed_on=now - timedelta(days=3),
+        )
+        updated_query = self.insert_query(
+            example_db.id,
+            admin.id,
+            client_id,
+            sql="SELECT col1, col2 from table1",
+            select_sql="SELECT col1, col2 from table1",
+            executed_sql="SELECT col1, col2 from table1 LIMIT 100",
+            changed_on=now - timedelta(days=1),
+        )
+
+        self.login(username="admin")
+        timestamp = datetime.timestamp(now - timedelta(days=2)) * 1000
+        uri = f"api/v1/query/updated_since?q={prison.dumps({'last_updated_ms': timestamp})}"
+        rv = self.client.get(uri)
+        self.assertEqual(rv.status_code, 200)
+
+        expected_result = updated_query.to_dict()
+        data = json.loads(rv.data.decode("utf-8"))
+        self.assertEqual(len(data["result"]), 1)
+        for key, value in data["result"][0].items():
+            # We can't assert timestamp
+            print(key)

Review Comment:
   this print is prob a debug leftover



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] commented on pull request #22611: chore: Migrate /superset/queries/ to API v1

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #22611:
URL: https://github.com/apache/superset/pull/22611#issuecomment-1372881579

   # [Codecov](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#22611](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8db4696) into [master](https://codecov.io/gh/apache/superset/commit/6e4d6e599bbbc99d9cb78c4e00d884f6291c50c9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6e4d6e5) will **decrease** coverage by `11.24%`.
   > The diff coverage is `73.52%`.
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #22611       +/-   ##
   ===========================================
   - Coverage   67.00%   55.76%   -11.25%     
   ===========================================
     Files        1859     1859               
     Lines       71019    71064       +45     
     Branches     7766     7766               
   ===========================================
   - Hits        47587    39626     -7961     
   - Misses      21410    29416     +8006     
     Partials     2022     2022               
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `?` | |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `52.39% <70.96%> (+0.02%)` | :arrow_up: |
   | python | `57.83% <77.41%> (-23.50%)` | :arrow_down: |
   | sqlite | `?` | |
   | unit | `51.49% <77.41%> (+0.02%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [superset/constants.py](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uc3RhbnRzLnB5) | `100.00% <ø> (ø)` | |
   | [...d/src/SqlLab/components/QueryAutoRefresh/index.tsx](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1F1ZXJ5QXV0b1JlZnJlc2gvaW5kZXgudHN4) | `70.83% <33.33%> (ø)` | |
   | [superset/queries/api.py](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcXVlcmllcy9hcGkucHk=) | `86.53% <69.56%> (-13.47%)` | :arrow_down: |
   | [superset/queries/dao.py](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcXVlcmllcy9kYW8ucHk=) | `100.00% <100.00%> (ø)` | |
   | [superset/queries/schemas.py](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcXVlcmllcy9zY2hlbWFzLnB5) | `100.00% <100.00%> (ø)` | |
   | [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `35.61% <100.00%> (-39.37%)` | :arrow_down: |
   | [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset/tags/core.py](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFncy9jb3JlLnB5) | `4.54% <0.00%> (-95.46%)` | :arrow_down: |
   | [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `0.00% <0.00%> (-90.91%)` | :arrow_down: |
   | [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `0.00% <0.00%> (-87.88%)` | :arrow_down: |
   | ... and [293 more](https://codecov.io/gh/apache/superset/pull/22611?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] diegomedina248 merged pull request #22611: chore: Migrate /superset/queries/ to API v1

Posted by "diegomedina248 (via GitHub)" <gi...@apache.org>.
diegomedina248 merged PR #22611:
URL: https://github.com/apache/superset/pull/22611


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] diegomedina248 commented on a diff in pull request #22611: chore: Migrate /superset/queries/ to API v1

Posted by GitBox <gi...@apache.org>.
diegomedina248 commented on code in PR #22611:
URL: https://github.com/apache/superset/pull/22611#discussion_r1068423718


##########
superset/queries/api.py:
##########
@@ -123,3 +142,56 @@ class QueryRestApi(BaseSupersetModelRestApi):
     base_related_field_filters = {"database": [["id", DatabaseFilter, lambda: []]]}
     allowed_rel_fields = {"database", "user"}
     allowed_distinct_fields = {"status"}
+
+    @expose("/updated_since")
+    @protect()
+    @safe
+    @rison(queries_get_updated_since_schema)
+    @statsd_metrics
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
+        f".get_updated_since",
+        log_to_statsd=False,
+    )
+    def get_updated_since(self, **kwargs: Any) -> FlaskResponse:
+        """Get a list of queries that changed after last_updated_ms
+        ---
+        get:
+          summary: Get a list of queries that changed after last_updated_ms
+          parameters:
+          - in: query
+            name: q
+            content:
+              application/json:
+                schema:
+                  $ref: '#/components/schemas/queries_get_updated_since_schema'
+          responses:
+            200:
+              description: Queries list
+              content:
+                application/json:
+                  schema:
+                    type: object
+                    properties:
+                      result:
+                        description: >-
+                          A List of queries that changed after last_updated_ms
+                        type: array
+                        items:
+                          $ref: '#/components/schemas/{{self.__class__.__name__}}.get'
+            400:
+              $ref: '#/components/responses/400'
+            401:
+              $ref: '#/components/responses/401'
+            404:
+              $ref: '#/components/responses/404'
+            500:
+              $ref: '#/components/responses/500'
+        """
+        try:
+            last_updated_ms = kwargs["rison"].get("last_updated_ms", 0)
+            queries = QueryDAO.get_queries_changed_after(last_updated_ms)
+            payload = [q.to_dict() for q in queries]

Review Comment:
   Having taking a closer look, here's what I see:
   
   The `to_dict` object returns a different payload, more fields and in a different format.
   The payload, as is, is returned by multiple endpoints, like `/superset/sql_json`, `superset/results`.
   This object shape is used throughout sql lab, in the actions, reducers, components. It even bleeds to `superset-ui-core`.
   
   The above, coupled with the sqllab not being fully typed would make this a big upgrade.
   I think that migration should be separate as to not block v1 migration.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] dpgaspar commented on a diff in pull request #22611: chore: Migrate /superset/queries/ to API v1

Posted by GitBox <gi...@apache.org>.
dpgaspar commented on code in PR #22611:
URL: https://github.com/apache/superset/pull/22611#discussion_r1063963277


##########
superset/queries/api.py:
##########
@@ -123,3 +142,56 @@ class QueryRestApi(BaseSupersetModelRestApi):
     base_related_field_filters = {"database": [["id", DatabaseFilter, lambda: []]]}
     allowed_rel_fields = {"database", "user"}
     allowed_distinct_fields = {"status"}
+
+    @expose("/updated_since")
+    @protect()
+    @safe
+    @rison(queries_get_updated_since_schema)
+    @statsd_metrics
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
+        f".get_updated_since",
+        log_to_statsd=False,
+    )
+    def get_updated_since(self, **kwargs: Any) -> FlaskResponse:
+        """Get a list of queries that changed after last_updated_ms
+        ---
+        get:
+          summary: Get a list of queries that changed after last_updated_ms
+          parameters:
+          - in: query
+            name: q
+            content:
+              application/json:
+                schema:
+                  $ref: '#/components/schemas/queries_get_updated_since_schema'
+          responses:
+            200:
+              description: Queries list
+              content:
+                application/json:
+                  schema:
+                    type: object
+                    properties:
+                      result:
+                        description: >-
+                          A List of queries that changed after last_updated_ms
+                        type: array
+                        items:
+                          $ref: '#/components/schemas/{{self.__class__.__name__}}.get'
+            400:
+              $ref: '#/components/responses/400'
+            401:
+              $ref: '#/components/responses/401'
+            404:
+              $ref: '#/components/responses/404'
+            500:
+              $ref: '#/components/responses/500'
+        """
+        try:
+            last_updated_ms = kwargs["rison"].get("last_updated_ms", 0)
+            queries = QueryDAO.get_queries_changed_after(last_updated_ms)
+            payload = [q.to_dict() for q in queries]

Review Comment:
   got it, I was assuming this was using the `to_dict` method on FAB. I vote for making the transition now.
   Actually I think we may not even need this endpoint at all, most probably the get list endpoint is able to perform this simple filter out of the box. I'll check



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] dpgaspar commented on a diff in pull request #22611: chore: Migrate /superset/queries/ to API v1

Posted by GitBox <gi...@apache.org>.
dpgaspar commented on code in PR #22611:
URL: https://github.com/apache/superset/pull/22611#discussion_r1063963277


##########
superset/queries/api.py:
##########
@@ -123,3 +142,56 @@ class QueryRestApi(BaseSupersetModelRestApi):
     base_related_field_filters = {"database": [["id", DatabaseFilter, lambda: []]]}
     allowed_rel_fields = {"database", "user"}
     allowed_distinct_fields = {"status"}
+
+    @expose("/updated_since")
+    @protect()
+    @safe
+    @rison(queries_get_updated_since_schema)
+    @statsd_metrics
+    @event_logger.log_this_with_context(
+        action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
+        f".get_updated_since",
+        log_to_statsd=False,
+    )
+    def get_updated_since(self, **kwargs: Any) -> FlaskResponse:
+        """Get a list of queries that changed after last_updated_ms
+        ---
+        get:
+          summary: Get a list of queries that changed after last_updated_ms
+          parameters:
+          - in: query
+            name: q
+            content:
+              application/json:
+                schema:
+                  $ref: '#/components/schemas/queries_get_updated_since_schema'
+          responses:
+            200:
+              description: Queries list
+              content:
+                application/json:
+                  schema:
+                    type: object
+                    properties:
+                      result:
+                        description: >-
+                          A List of queries that changed after last_updated_ms
+                        type: array
+                        items:
+                          $ref: '#/components/schemas/{{self.__class__.__name__}}.get'
+            400:
+              $ref: '#/components/responses/400'
+            401:
+              $ref: '#/components/responses/401'
+            404:
+              $ref: '#/components/responses/404'
+            500:
+              $ref: '#/components/responses/500'
+        """
+        try:
+            last_updated_ms = kwargs["rison"].get("last_updated_ms", 0)
+            queries = QueryDAO.get_queries_changed_after(last_updated_ms)
+            payload = [q.to_dict() for q in queries]

Review Comment:
   got it, I was assuming this was using the `to_dict` method on FAB. I vote for making the transition now.
   Actually I think we may not even need this endpoint at all, what do you think about making a new filter and leverage the get list endpoint?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org