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 2022/01/26 02:19:21 UTC
[GitHub] [superset] ad-m opened a new pull request #18170: refactor: extract json_required view decorator
ad-m opened a new pull request #18170:
URL: https://github.com/apache/superset/pull/18170
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
During review #18151 there was a comment about code duplication of snippet like:
```python
if not request.is_json:
raise InvalidPayloadFormatError("Request is not JSON")
```
> Do we have a decorator for this? Might want to add one if not yet.
_Originally posted by @ktmud in https://github.com/apache/superset/pull/18151#discussion_r791180093_
I would like to get familiar with part of the codebase of the project written in Python, so I perceived that as a good first issue for a new contributor.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
N/A
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
Python test handle all scenarios. I added a new one integration tests to verify behaviour of extracted decorator.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue: #18151
- [ ] Required feature flags: NO
- [ ] Changes UI: NO
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)): NO
- [ ] 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: NO
- [ ] Removes existing feature or API: NO
@ktmud, @michael-s-molina Can I request reviews? I am aware this is a trivial change, but I want to try to grab the project style & structure to be able to make bigger changes, and the beginning has to be somewhere.
--
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] ad-m commented on pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
ad-m commented on pull request #18170:
URL: https://github.com/apache/superset/pull/18170#issuecomment-1022108096
I like to make it even more generic. I will apply you suggestion soon!
śr., 26 sty 2022, 12:21 użytkownik Michael S. Molina <
***@***.***> napisał:
> Thank you for the PR @ad-m <https://github.com/ad-m>. Grabbing an example
> from Java, I really like the abstraction of @Produces and @Consumes
> annotations of JAX-RS
> <https://docs.oracle.com/cd/E19798-01/821-1841/gipzh/index.html>. They
> accept the MIME type as an argument.
>
> @Consumes("application/json")
>
> Since request.is_json
> <https://flask.palletsprojects.com/en/2.0.x/api/#flask.Request.is_json>
> is pretty much checking the MIME type we could create a generic decorator
> the same way JAX-RS does. This will allow us to validate all request types
> in our endpoints.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/superset/pull/18170#issuecomment-1022106122>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AA3TNL3EOCQQLMKJOC56K4LUX7KLRANCNFSM5MZ3RLVA>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
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] michael-s-molina edited a comment on pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
michael-s-molina edited a comment on pull request #18170:
URL: https://github.com/apache/superset/pull/18170#issuecomment-1022106122
Thank you for the PR @ad-m. Grabbing an example from Java, I really like the abstraction of `@Produces` and `@Consumes` annotations of [JAX-RS](https://docs.oracle.com/cd/E19798-01/821-1841/gipzh/index.html). They accept the MIME type as an argument.
`@Consumes("application/json")`
Since [request.is_json](https://flask.palletsprojects.com/en/2.0.x/api/#flask.Request.is_json) is pretty much checking the MIME type we could create a generic decorator the same way JAX-RS does. This will allow us to validate all request types in our endpoints.
It would also be a good idea to keep the MIME types as constants if Python/Flask does not provide them yet.
--
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] edited a comment on pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18170:
URL: https://github.com/apache/superset/pull/18170#issuecomment-1023378034
# [Codecov](https://codecov.io/gh/apache/superset/pull/18170?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 [#18170](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ee92c54) into [master](https://codecov.io/gh/apache/superset/commit/a4e93afdbf5d5c28a77619ef3085406bfccf9775?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a4e93af) will **increase** coverage by `0.02%`.
> The diff coverage is `97.91%`.
> :exclamation: Current head ee92c54 differs from pull request most recent head 00b2d09. Consider uploading reports for the commit 00b2d09 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18170 +/- ##
==========================================
+ Coverage 66.05% 66.08% +0.02%
==========================================
Files 1591 1591
Lines 62418 62413 -5
Branches 6286 6286
==========================================
+ Hits 41228 41243 +15
+ Misses 19568 19548 -20
Partials 1622 1622
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.29% <87.50%> (+0.11%)` | :arrow_up: |
| mysql | `81.34% <97.91%> (+0.06%)` | :arrow_up: |
| postgres | `81.39% <97.91%> (+0.06%)` | :arrow_up: |
| presto | `52.13% <87.50%> (+0.11%)` | :arrow_up: |
| python | `81.83% <97.91%> (+0.06%)` | :arrow_up: |
| sqlite | `81.09% <97.91%> (+0.06%)` | :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/18170?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/views/base\_api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvYmFzZV9hcGkucHk=) | `97.89% <92.85%> (-0.33%)` | :arrow_down: |
| [superset/annotation\_layers/annotations/api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvYW5ub3RhdGlvbl9sYXllcnMvYW5ub3RhdGlvbnMvYXBpLnB5) | `89.31% <100.00%> (+1.34%)` | :arrow_up: |
| [superset/annotation\_layers/api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvYW5ub3RhdGlvbl9sYXllcnMvYXBpLnB5) | `86.32% <100.00%> (+1.45%)` | :arrow_up: |
| [superset/charts/api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY2hhcnRzL2FwaS5weQ==) | `85.93% <100.00%> (+0.70%)` | :arrow_up: |
| [superset/dashboards/api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9hcGkucHk=) | `92.54% <100.00%> (+0.65%)` | :arrow_up: |
| [superset/dashboards/filter\_sets/api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9hcGkucHk=) | `99.02% <100.00%> (+1.88%)` | :arrow_up: |
| [superset/databases/api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `93.99% <100.00%> (+0.96%)` | :arrow_up: |
| [superset/datasets/api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvYXBpLnB5) | `91.89% <100.00%> (+0.86%)` | :arrow_up: |
| [superset/explore/form\_data/api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhwbG9yZS9mb3JtX2RhdGEvYXBpLnB5) | `95.74% <100.00%> (+1.99%)` | :arrow_up: |
| [superset/key\_value/api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2FwaS5weQ==) | `81.52% <100.00%> (+1.73%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a4e93af...00b2d09](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov[bot] edited a comment on pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18170:
URL: https://github.com/apache/superset/pull/18170#issuecomment-1023378034
# [Codecov](https://codecov.io/gh/apache/superset/pull/18170?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 [#18170](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ee92c54) into [master](https://codecov.io/gh/apache/superset/commit/4ad5ad045a9adb506d14b2c02fdbefc564d25bdb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ad5ad0) will **increase** coverage by `0.03%`.
> The diff coverage is `91.56%`.
> :exclamation: Current head ee92c54 differs from pull request most recent head 677693a. Consider uploading reports for the commit 677693a to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18170 +/- ##
==========================================
+ Coverage 66.04% 66.08% +0.03%
==========================================
Files 1591 1591
Lines 62398 62413 +15
Branches 6283 6286 +3
==========================================
+ Hits 41210 41243 +33
+ Misses 19567 19548 -19
- Partials 1621 1622 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.29% <77.27%> (+0.10%)` | :arrow_up: |
| mysql | `81.34% <98.48%> (+0.07%)` | :arrow_up: |
| postgres | `81.39% <98.48%> (+0.07%)` | :arrow_up: |
| presto | `52.13% <77.27%> (+0.10%)` | :arrow_up: |
| python | `81.83% <98.48%> (+0.07%)` | :arrow_up: |
| sqlite | `81.09% <98.48%> (+0.07%)` | :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/18170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-echarts/src/MixedTimeseries/controlPanel.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL2NvbnRyb2xQYW5lbC50c3g=) | `80.00% <ø> (ø)` | |
| [...hart-echarts/src/MixedTimeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL3RyYW5zZm9ybVByb3BzLnRz) | `0.00% <0.00%> (ø)` | |
| [...lugin-chart-echarts/src/Timeseries/transformers.ts](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1lcnMudHM=) | `50.83% <ø> (ø)` | |
| [...c/views/CRUD/data/database/DatabaseModal/index.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL2luZGV4LnRzeA==) | `32.46% <0.00%> (-0.22%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/18170/diff?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==) | `77.74% <ø> (ø)` | |
| [...l/AdhocFilterEditPopoverSimpleTabContent/index.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXJTaW1wbGVUYWJDb250ZW50L2luZGV4LnRzeA==) | `65.07% <33.33%> (ø)` | |
| [...et-frontend/src/components/TableSelector/index.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVTZWxlY3Rvci9pbmRleC50c3g=) | `62.65% <50.00%> (-0.32%)` | :arrow_down: |
| [superset/views/base\_api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvYmFzZV9hcGkucHk=) | `97.89% <92.85%> (-0.33%)` | :arrow_down: |
| [...frontend/src/components/DatabaseSelector/index.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YWJhc2VTZWxlY3Rvci9pbmRleC50c3g=) | `88.70% <100.00%> (+0.37%)` | :arrow_up: |
| [...ashboard/components/UndoRedoKeyListeners/index.jsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1VuZG9SZWRvS2V5TGlzdGVuZXJzL2luZGV4LmpzeA==) | `100.00% <100.00%> (ø)` | |
| ... and [21 more](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4ad5ad0...677693a](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] ad-m commented on pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
ad-m commented on pull request #18170:
URL: https://github.com/apache/superset/pull/18170#issuecomment-1023646229
I renamed decorator to `requires_json` and created a separate decorator `requires_form_data` for `multipart/form_data` request.
I inspected code looking for other request formats and I notice that have `superset.views.core.Superset.import_dashboards` (endpoint `/import_dashboard/`) which uses two request formats depending on the request method - empty request (GET with HTML form in response) or form-data (POST with redirection on success response). However, I think we can handle this particular scenario in a special way.
I decided to use two decorators because I considered a small number of scenarios, I felt there was no reason to add more responsibility to one function (which increases complexity) since there are only two simple scenarios. The current approach is simple - readable and easy to use.
--
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] edited a comment on pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #18170:
URL: https://github.com/apache/superset/pull/18170#issuecomment-1023378034
# [Codecov](https://codecov.io/gh/apache/superset/pull/18170?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 [#18170](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ee92c54) into [master](https://codecov.io/gh/apache/superset/commit/4ad5ad045a9adb506d14b2c02fdbefc564d25bdb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ad5ad0) will **increase** coverage by `0.01%`.
> The diff coverage is `91.56%`.
> :exclamation: Current head ee92c54 differs from pull request most recent head 677693a. Consider uploading reports for the commit 677693a to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18170 +/- ##
==========================================
+ Coverage 66.04% 66.05% +0.01%
==========================================
Files 1591 1591
Lines 62398 62413 +15
Branches 6283 6286 +3
==========================================
+ Hits 41210 41227 +17
+ Misses 19567 19564 -3
- Partials 1621 1622 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.29% <77.27%> (+0.10%)` | :arrow_up: |
| mysql | `81.34% <98.48%> (+0.07%)` | :arrow_up: |
| postgres | `?` | |
| presto | `52.13% <77.27%> (+0.10%)` | :arrow_up: |
| python | `81.78% <98.48%> (+0.02%)` | :arrow_up: |
| sqlite | `81.09% <98.48%> (+0.07%)` | :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/18170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-echarts/src/MixedTimeseries/controlPanel.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL2NvbnRyb2xQYW5lbC50c3g=) | `80.00% <ø> (ø)` | |
| [...hart-echarts/src/MixedTimeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL3RyYW5zZm9ybVByb3BzLnRz) | `0.00% <0.00%> (ø)` | |
| [...lugin-chart-echarts/src/Timeseries/transformers.ts](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1lcnMudHM=) | `50.83% <ø> (ø)` | |
| [...c/views/CRUD/data/database/DatabaseModal/index.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL2luZGV4LnRzeA==) | `32.46% <0.00%> (-0.22%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/18170/diff?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==) | `77.74% <ø> (ø)` | |
| [...l/AdhocFilterEditPopoverSimpleTabContent/index.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXJTaW1wbGVUYWJDb250ZW50L2luZGV4LnRzeA==) | `65.07% <33.33%> (ø)` | |
| [...et-frontend/src/components/TableSelector/index.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVTZWxlY3Rvci9pbmRleC50c3g=) | `62.65% <50.00%> (-0.32%)` | :arrow_down: |
| [superset/views/base\_api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvYmFzZV9hcGkucHk=) | `97.47% <92.85%> (-0.75%)` | :arrow_down: |
| [...frontend/src/components/DatabaseSelector/index.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YWJhc2VTZWxlY3Rvci9pbmRleC50c3g=) | `88.70% <100.00%> (+0.37%)` | :arrow_up: |
| [...ashboard/components/UndoRedoKeyListeners/index.jsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1VuZG9SZWRvS2V5TGlzdGVuZXJzL2luZGV4LmpzeA==) | `100.00% <100.00%> (ø)` | |
| ... and [28 more](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4ad5ad0...677693a](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov[bot] commented on pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #18170:
URL: https://github.com/apache/superset/pull/18170#issuecomment-1023378034
# [Codecov](https://codecov.io/gh/apache/superset/pull/18170?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 [#18170](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ee92c54) into [master](https://codecov.io/gh/apache/superset/commit/4ad5ad045a9adb506d14b2c02fdbefc564d25bdb?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ad5ad0) will **increase** coverage by `0.01%`.
> The diff coverage is `91.56%`.
> :exclamation: Current head ee92c54 differs from pull request most recent head 677693a. Consider uploading reports for the commit 677693a to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18170 +/- ##
==========================================
+ Coverage 66.04% 66.05% +0.01%
==========================================
Files 1591 1591
Lines 62398 62413 +15
Branches 6283 6286 +3
==========================================
+ Hits 41210 41227 +17
+ Misses 19567 19564 -3
- Partials 1621 1622 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.29% <77.27%> (+0.10%)` | :arrow_up: |
| mysql | `81.34% <98.48%> (+0.07%)` | :arrow_up: |
| postgres | `?` | |
| presto | `52.13% <77.27%> (+0.10%)` | :arrow_up: |
| python | `81.78% <98.48%> (+0.02%)` | :arrow_up: |
| sqlite | `81.09% <98.48%> (+0.07%)` | :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/18170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...chart-echarts/src/MixedTimeseries/controlPanel.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL2NvbnRyb2xQYW5lbC50c3g=) | `80.00% <ø> (ø)` | |
| [...hart-echarts/src/MixedTimeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL3RyYW5zZm9ybVByb3BzLnRz) | `0.00% <0.00%> (ø)` | |
| [...lugin-chart-echarts/src/Timeseries/transformers.ts](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1lcnMudHM=) | `50.83% <ø> (ø)` | |
| [...c/views/CRUD/data/database/DatabaseModal/index.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL2luZGV4LnRzeA==) | `32.46% <0.00%> (-0.22%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/18170/diff?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==) | `77.74% <ø> (ø)` | |
| [...l/AdhocFilterEditPopoverSimpleTabContent/index.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXJTaW1wbGVUYWJDb250ZW50L2luZGV4LnRzeA==) | `65.07% <33.33%> (ø)` | |
| [...et-frontend/src/components/TableSelector/index.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVTZWxlY3Rvci9pbmRleC50c3g=) | `62.65% <50.00%> (-0.32%)` | :arrow_down: |
| [superset/views/base\_api.py](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvYmFzZV9hcGkucHk=) | `97.47% <92.85%> (-0.75%)` | :arrow_down: |
| [...frontend/src/components/DatabaseSelector/index.tsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YWJhc2VTZWxlY3Rvci9pbmRleC50c3g=) | `88.70% <100.00%> (+0.37%)` | :arrow_up: |
| [...ashboard/components/UndoRedoKeyListeners/index.jsx](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1VuZG9SZWRvS2V5TGlzdGVuZXJzL2luZGV4LmpzeA==) | `100.00% <100.00%> (ø)` | |
| ... and [28 more](https://codecov.io/gh/apache/superset/pull/18170/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4ad5ad0...677693a](https://codecov.io/gh/apache/superset/pull/18170?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] michael-s-molina commented on pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #18170:
URL: https://github.com/apache/superset/pull/18170#issuecomment-1023296635
> I'm not sure we need to make it generic. Which other request payload type we must handle?
The `*/import` endpoints use `multipart/form_data` as the MIME type for the body.
> I'd recommend use a single decorator to keep things simple---so you don't have to import both the decorator and the constant. It may be more appropriate to name the function with a verb, i.e. requires_json instead of json_required (which sounds like a state).
While checking the API docs I found `application/json` and `multipart/form_data` as request MIME types. I also find `application/text` as the response MIME type for some endpoints. There doesn't seem to be much variation, so I'm fine with either approach.
--
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] michael-s-molina commented on pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #18170:
URL: https://github.com/apache/superset/pull/18170#issuecomment-1022106122
Thank you for the PR @ad-m. Grabbing an example from Java, I really like the abstraction of `@Produces` and `@Consumes` annotations of [JAX-RS](https://docs.oracle.com/cd/E19798-01/821-1841/gipzh/index.html). They accept the MIME type as an argument.
`@Consumes("application/json")`
Since [request.is_json](https://flask.palletsprojects.com/en/2.0.x/api/#flask.Request.is_json) is pretty much checking the MIME type we could create a generic decorator the same way JAX-RS does. This will allow us to validate all request types in our endpoints.
--
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] ktmud commented on pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #18170:
URL: https://github.com/apache/superset/pull/18170#issuecomment-1022370971
I'm not sure we need to make it generic. Which other request payload type we must handle?
--
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] ktmud commented on pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #18170:
URL: https://github.com/apache/superset/pull/18170#issuecomment-1023371689
I think eventually we would want everything to be submitted via JSON, except maybe when uploading files, in which case you can just not use this decorator.
--
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] michael-s-molina merged pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
michael-s-molina merged pull request #18170:
URL: https://github.com/apache/superset/pull/18170
--
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] ktmud commented on pull request #18170: refactor: extract json_required view decorator
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #18170:
URL: https://github.com/apache/superset/pull/18170#issuecomment-1022380849
I'd recommend use a single decorator to keep things simple---so you don't have to import both the decorator and the constant. It may be more appropriate to name the function with a verb, i.e. `requires_json` instead of `json_required` (which sounds like a state).
--
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