-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
Fetch charts with GET to benefit from browser cache and conditional requests #7032
Merged
+552
−370
Merged
Changes from 1 commit
Commits
Show all changes
91 commits
Select commit
Hold shift + click to select a range
1ef7fb6
Sparkline dates aren't formatting in Time Series Table (#6976)
khtruong e7d97db
Fix the white background shown in SQL editor on drag (#7021)
xtinec e6886fb
Show tooltip with time frame (#6979)
betodealmeida b3966e4
Fix time filter control (#6978)
betodealmeida f4e3923
Enhancement of query context and object. (#6962)
conglei d7b2c3e
[fix] /superset/slice/id url is too long (#6989)
9855837
[WIP] fix user specified JSON metadata not updating dashboard on refr…
thunter009 6d0c390
feat: add ability to change font size in big number (#7003)
khtruong 0e1a3d3
Allow LIMIT to be specified in parameters (#7052)
betodealmeida 0c65b3f
[fix] Cursor jumping when editing chart and dashboard titles (#7038)
c65a166
Changing time table viz to pass formatTime a date (#7020)
michellethomas 771d212
[db-engine-spec] Aligning Hive/Presto partition logic (#7007)
john-bodley fa73a8d
[fix] explore chart from dashboard missed slice title (#7046)
8d6e3f1
fix inaccurate data calculation with adata rolling and contribution (…
conglei 6d0752c
Adding warning message for sqllab save query (#7028)
michellethomas bd2a8b5
[datasource] Ensuring consistent behavior of datasource editing/savin…
john-bodley 81a1287
[csv-upload] Fixing message encoding (#6971)
john-bodley 6a7d5fc
[sql-parse] Fixing LIMIT exceptions (#6963)
john-bodley ee85089
Adding custom control overrides (#6956)
michellethomas 7bc3006
[sqlparse] Fixing table name extraction for ill-defined query (#7029)
john-bodley 4ffc1b3
[missing values] Removing replacing missing values (#4905)
john-bodley dfc0010
[SQL Lab] Improved query and results tabs rendering reliability (#7082)
enricoberti 98cf417
Fix filter_box migration PR #6523 (#7066)
mistercrunch 959199d
SQL editor layout makeover (#7102)
xtinec b599855
[forms] Fix handling of NULLs
9d21f70
handle null column_name in sqla and druid models
bdbb354
Use metric name instead of metric in filter box (#7106)
michellethomas 87fb2df
Bump python lib croniter to an existing version (#7132)
mistercrunch b7fb15f
Revert PR #6933 (#7162)
betodealmeida 63f98dc
Add decorator for etag cache
betodealmeida b65dca6
Fetch charts with GET
betodealmeida a70a2f4
Small fixes
betodealmeida afabd44
Fix typo
betodealmeida c49dc70
Compute correct cache key; fix logging
betodealmeida ac07631
Check perms on cached response
betodealmeida ea26a75
Revert change
betodealmeida 9d4acf7
If perms fail, return naked response
betodealmeida 2a46132
Fix lint
betodealmeida c65a0dd
Compute cache key from all form data
betodealmeida 35a7868
Pass extra_filters in GET request
betodealmeida 4dd19e0
Fix pylint
betodealmeida debd5e0
Fix flake8
betodealmeida e49cd03
Use ETags even if no cache is set
betodealmeida ca65c0c
Handle adhoc filters
betodealmeida 58c84db
Raise in debug mode
betodealmeida 60e849b
Rename actions
betodealmeida a913a84
Fix integration tests
betodealmeida 8ed131d
Do POST request on new charts
betodealmeida e96784a
Set extra/adhoc filters only in GET requests
betodealmeida 3f49312
Raise if check_perms fails
betodealmeida 532dc2e
Refactor auth
betodealmeida 2631325
Fix flake8
betodealmeida 7fb1931
Fix js unit tests
betodealmeida cc15133
Fix js unit tests that fail in lyftga
betodealmeida cb1e7cb
Fix js
betodealmeida 2f97bf9
Sparkline dates aren't formatting in Time Series Table (#6976)
khtruong 32088cc
Changing time table viz to pass formatTime a date (#7020)
michellethomas 23dadc7
SQL editor layout makeover (#7102)
xtinec dc59507
Add decorator for etag cache
betodealmeida a745f32
Fetch charts with GET
betodealmeida 1931441
Small fixes
betodealmeida ff037e5
Fix typo
betodealmeida 6a4adb3
Compute correct cache key; fix logging
betodealmeida 9436034
Check perms on cached response
betodealmeida 9df4dc2
Revert change
betodealmeida e0fbb64
If perms fail, return naked response
betodealmeida c093b03
Fix lint
betodealmeida ef3afd2
Compute cache key from all form data
betodealmeida 86e4b93
Pass extra_filters in GET request
betodealmeida 68412bf
Fix pylint
betodealmeida 69eeb1d
Fix flake8
betodealmeida fe8cc65
Use ETags even if no cache is set
betodealmeida f8200a1
Handle adhoc filters
betodealmeida b3c488d
Raise in debug mode
betodealmeida 89f0192
Rename actions
betodealmeida 3324f15
Fix integration tests
betodealmeida c0caac8
Do POST request on new charts
betodealmeida 4b9ffa7
Set extra/adhoc filters only in GET requests
betodealmeida b75e11e
Raise if check_perms fails
betodealmeida 96e3c9b
Refactor auth
betodealmeida a1fdb44
Fix flake8
betodealmeida d0bff86
Fix js unit tests
betodealmeida 057b953
Fix js unit tests that fail in lyftga
betodealmeida 7da0157
Fix js
betodealmeida ce17182
Merge
betodealmeida a364194
Fix bad merge
betodealmeida 5e418ed
Use far future when max_age=0
betodealmeida 407a87f
Merge
betodealmeida b48c57e
Merge branch 'lyft-VIZ-334' into VIZ-334
betodealmeida cb8cc64
Fix conflict
betodealmeida 67e5980
Merge branch 'lyft-VIZ-334' into VIZ-334
betodealmeida File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Raise in debug mode
commit b3c488dc5817fee58823ee7b8eec9e114a73845d
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe there is merit in following the pattern in Flask-Caching per here where the error is re-raised in debug mode. Note by default it also logs a fairly generic exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good point. I'll do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the pylint exception while we're in here? I'm not seeing that anywhere else in the codebase despite similarly structured except blocks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DiggidyDave the reason you're probably not seeing it is we have this defined in a number of files. It's a place holder when we enforced that new code could not have any
pylint
warnings.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, thanks for the info. If this is the first new code that isn't grandfathered into a whole file disable directive, should we consider either 1) making this adhere to the linter rules (ie don't disable it here) if this is a warning we care about or 2) adding a blanket exclusion to
.pylintrc
if this is a warning we never care about (which seems to be the case in practice)?If this is something that we globally don't care about at this particular moment, it seems like a good idea to just codify and enforce that in one place in the central config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DiggidyDave catching a broad exception is generally bad hence why the
pylint
rule exists, i.e.,is preferred over,
That said there are rare occasions when using the later form make sense, i.e., i) the caller may be agnostic of the underlying function being called, or ii) all exceptions need to correctly corralled. In this example I think the current logic is correct. Additionally it mimics Flask-Caching.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand why they are bad, and I agree this is ok here. However, linter rules that are always ignored in an ad-hoc way (ie not configured globally) sometimes can raise a red flag wrt code hygiene for me, so I thought I'd bring it up here while I'm ramping up on the codebase. :-) Thanks for your responses! I think we're on the same page.