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 2021/01/13 09:33:39 UTC
[GitHub] [superset] zhaoyongjie opened a new pull request #12489: fix: make pyparsing thread safe
zhaoyongjie opened a new pull request #12489:
URL: https://github.com/apache/superset/pull/12489
### SUMMARY
make `pyparsing` thread-safe
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
<!--- What steps should be taken to 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:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] 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.
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] junlincc edited a comment on pull request #12489: fix: make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759335627
✅
thanks for the fix!!
**With lock**
<img width="1785" alt="Screen Shot 2021-01-13 at 1 50 14 AM" src="https://user-images.githubusercontent.com/67837651/104435891-c6b64000-5541-11eb-975c-14ff0d65eaf2.png">
**Without lock(refreshing page)**
<img width="1767" alt="Screen Shot 2021-01-13 at 1 49 47 AM" src="https://user-images.githubusercontent.com/67837651/104435875-c0c05f00-5541-11eb-8c30-c266f6f6169e.png">
<img width="1763" alt="Screen Shot 2021-01-13 at 1 56 55 AM" src="https://user-images.githubusercontent.com/67837651/104436596-a76be280-5542-11eb-833a-b59ecf5aef4e.png">
<img width="1770" alt="Screen Shot 2021-01-13 at 1 56 46 AM" src="https://user-images.githubusercontent.com/67837651/104436600-a89d0f80-5542-11eb-9949-6c4018419e44.png">
<img width="887" alt="Screen Shot 2021-01-13 at 2 02 05 AM" src="https://user-images.githubusercontent.com/67837651/104437217-5f00f480-5543-11eb-9d49-9dfd07cac2bf.png">
----------------------------------------------------------------
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.
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] zhaoyongjie commented on pull request #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759401118
> Looks like packrat cache comes with a lock for free: https://github.com/pyparsing/pyparsing/blob/f10fcf8c98ea07a63f8286288ce82f773e7e152e/pyparsing/core.py#L719
>
> So I'm 90% sure this will fix the bug... (with the additional benefit of making things a little faster).
I changed this PR to add ParserElement.enablePackrat() and enable cache for datetime_parser, thank you for your finger out this solution
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759358284
# [Codecov](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=h1) Report
> Merging [#12489](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=desc) (fb1a96a) into [master](https://codecov.io/gh/apache/superset/commit/e9d66e904f9dfdaab26ee3cc208fbcfddbed1df9?el=desc) (e9d66e9) will **decrease** coverage by `2.35%`.
> The diff coverage is `100.00%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12489/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12489 +/- ##
==========================================
- Coverage 66.12% 63.77% -2.36%
==========================================
Files 1015 486 -529
Lines 49554 29920 -19634
Branches 5079 0 -5079
==========================================
- Hits 32767 19080 -13687
+ Misses 16647 10840 -5807
+ Partials 140 0 -140
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.77% <100.00%> (-0.32%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/utils/date\_parser.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGF0ZV9wYXJzZXIucHk=) | `95.13% <100.00%> (+0.02%)` | :arrow_up: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `96.42% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.86% <0.00%> (-2.99%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `72.89% <0.00%> (-2.48%)` | :arrow_down: |
| ... and [539 more](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=footer). Last update [e9d66e9...fb1a96a](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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 pull request #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759362301
right @junlincc we should figure out a way to replace the dependency (without removing the feature) not sure if our dependency is light or not. But this needs a more detailed analysis.
I may be wrong but there is the probability that this lock will not be the end of this problem
----------------------------------------------------------------
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.
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] junlincc edited a comment on pull request #12489: fix: make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759335627
✅
thanks for the fix!!
**With lock**
<img width="1785" alt="Screen Shot 2021-01-13 at 1 50 14 AM" src="https://user-images.githubusercontent.com/67837651/104435891-c6b64000-5541-11eb-975c-14ff0d65eaf2.png">
**Without lock(consistently getting error by refreshing page)**
<img width="1767" alt="Screen Shot 2021-01-13 at 1 49 47 AM" src="https://user-images.githubusercontent.com/67837651/104435875-c0c05f00-5541-11eb-8c30-c266f6f6169e.png">
<img width="1763" alt="Screen Shot 2021-01-13 at 1 56 55 AM" src="https://user-images.githubusercontent.com/67837651/104436596-a76be280-5542-11eb-833a-b59ecf5aef4e.png">
<img width="1770" alt="Screen Shot 2021-01-13 at 1 56 46 AM" src="https://user-images.githubusercontent.com/67837651/104436600-a89d0f80-5542-11eb-9949-6c4018419e44.png">
<img width="887" alt="Screen Shot 2021-01-13 at 2 02 05 AM" src="https://user-images.githubusercontent.com/67837651/104437217-5f00f480-5543-11eb-9d49-9dfd07cac2bf.png">
----------------------------------------------------------------
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.
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 #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759374475
I think I may have found another fix:
```diff
diff --git a/superset/utils/date_parser.py b/superset/utils/date_parser.py
index aee2c83a0..513c740ab 100644
--- a/superset/utils/date_parser.py
+++ b/superset/utils/date_parser.py
@@ -33,6 +33,7 @@ from pyparsing import (
Optional as ppOptional,
ParseException,
ParseResults,
+ ParserElement,
pyparsing_common,
quotedString,
Suppress,
@@ -40,6 +41,8 @@ from pyparsing import (
from .core import memoized
+ParserElement.enablePackrat()
+
logger = logging.getLogger(__name__)
@@ -375,7 +378,7 @@ class EvalHolidayFunc: # pylint: disable=too-few-public-methods
raise ValueError(_("Unable to find such a holiday: [{}]").format(holiday))
-@memoized()
+@memoized
def datetime_parser() -> ParseResults: # pylint: disable=too-many-locals
( # pylint: disable=invalid-name
DATETIME,
```
Could you test this diff and see if it fixes things for you?
----------------------------------------------------------------
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.
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 #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759379459
Looks like packrat cache comes with a lock for free: https://github.com/pyparsing/pyparsing/blob/f10fcf8c98ea07a63f8286288ce82f773e7e152e/pyparsing/core.py#L719
So I'm 90% sure this will fix the bug... (with the additional benefit of making things a little faster).
----------------------------------------------------------------
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.
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] junlincc edited a comment on pull request #12489: fix: make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759350449
> Thank you for the fix, but if pyparser is not thread safe we should start working on removing this dependency
I will hate to see this new feature go, so will many other users, as we already receive some positive feedback from the community. we should figure out a way to fix it forward instead of simply removing the dependency, right? @dpgaspar
----------------------------------------------------------------
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.
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 #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759376214
Enabling [packrat cache](https://pyparsing-docs.readthedocs.io/en/latest/pyparsing.html#pyparsing.ParserElement.enablePackrat) seems to have fixed it for me.
I'd assume no other programs are using `pyparsing` so this should be relatively safe.
----------------------------------------------------------------
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.
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] villebro edited a comment on pull request #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
villebro edited a comment on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759385906
> Looks like packrat cache comes with a lock for free: https://github.com/pyparsing/pyparsing/blob/f10fcf8c98ea07a63f8286288ce82f773e7e152e/pyparsing/core.py#L719
>
> So I'm 90% sure this will fix the bug... (with the additional benefit of making things a little faster).
I can confirm that this fixes the problem for me, too 👏 This looks like the perfect solution for this case.
----------------------------------------------------------------
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.
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] junlincc commented on pull request #12489: fix: make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759335627
lgtm!
**Without lock**
<img width="1767" alt="Screen Shot 2021-01-13 at 1 49 47 AM" src="https://user-images.githubusercontent.com/67837651/104435875-c0c05f00-5541-11eb-8c30-c266f6f6169e.png">
**With lock**
<img width="1785" alt="Screen Shot 2021-01-13 at 1 50 14 AM" src="https://user-images.githubusercontent.com/67837651/104435891-c6b64000-5541-11eb-975c-14ff0d65eaf2.png">
----------------------------------------------------------------
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.
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] junlincc commented on pull request #12489: fix: make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759350449
> Thank you for the fix, but if pyparser is not thread safe we should start working on removing this dependency
I will hate to see this new feature go, so will many other users, as we already receive some positive feedback from the community. can we figure out a way to fix it forward instead of removing the dependency? @dpgaspar
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759358284
# [Codecov](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=h1) Report
> Merging [#12489](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=desc) (fb1a96a) into [master](https://codecov.io/gh/apache/superset/commit/e9d66e904f9dfdaab26ee3cc208fbcfddbed1df9?el=desc) (e9d66e9) will **decrease** coverage by `7.09%`.
> The diff coverage is `100.00%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12489/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12489 +/- ##
==========================================
- Coverage 66.12% 59.02% -7.10%
==========================================
Files 1015 959 -56
Lines 49554 46791 -2763
Branches 5079 4341 -738
==========================================
- Hits 32767 27620 -5147
- Misses 16647 19171 +2524
+ Partials 140 0 -140
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.61% <ø> (+5.11%)` | :arrow_up: |
| javascript | `?` | |
| python | `63.77% <100.00%> (-0.32%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/utils/date\_parser.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGF0ZV9wYXJzZXIucHk=) | `95.13% <100.00%> (+0.02%)` | :arrow_up: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...end/src/SqlLab/components/ExploreResultsButton.jsx](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVSZXN1bHRzQnV0dG9uLmpzeA==) | `8.00% <0.00%> (-84.00%)` | :arrow_down: |
| ... and [415 more](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=footer). Last update [e9d66e9...fb1a96a](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] junlincc edited a comment on pull request #12489: fix: make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759335627
**Without lock**
<img width="1767" alt="Screen Shot 2021-01-13 at 1 49 47 AM" src="https://user-images.githubusercontent.com/67837651/104435875-c0c05f00-5541-11eb-8c30-c266f6f6169e.png">
**With lock**
<img width="1785" alt="Screen Shot 2021-01-13 at 1 50 14 AM" src="https://user-images.githubusercontent.com/67837651/104435891-c6b64000-5541-11eb-975c-14ff0d65eaf2.png">
----------------------------------------------------------------
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.
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] junlincc edited a comment on pull request #12489: fix: make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759335627
✅
thanks for the fix!!
**With lock**
<img width="1785" alt="Screen Shot 2021-01-13 at 1 50 14 AM" src="https://user-images.githubusercontent.com/67837651/104435891-c6b64000-5541-11eb-975c-14ff0d65eaf2.png">
**Without lock(constantly refreshing page)**
<img width="1767" alt="Screen Shot 2021-01-13 at 1 49 47 AM" src="https://user-images.githubusercontent.com/67837651/104435875-c0c05f00-5541-11eb-8c30-c266f6f6169e.png">
<img width="1763" alt="Screen Shot 2021-01-13 at 1 56 55 AM" src="https://user-images.githubusercontent.com/67837651/104436596-a76be280-5542-11eb-833a-b59ecf5aef4e.png">
<img width="1770" alt="Screen Shot 2021-01-13 at 1 56 46 AM" src="https://user-images.githubusercontent.com/67837651/104436600-a89d0f80-5542-11eb-9949-6c4018419e44.png">
<img width="887" alt="Screen Shot 2021-01-13 at 2 02 05 AM" src="https://user-images.githubusercontent.com/67837651/104437217-5f00f480-5543-11eb-9d49-9dfd07cac2bf.png">
----------------------------------------------------------------
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.
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-io commented on pull request #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759358284
# [Codecov](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=h1) Report
> Merging [#12489](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=desc) (fb1a96a) into [master](https://codecov.io/gh/apache/superset/commit/e9d66e904f9dfdaab26ee3cc208fbcfddbed1df9?el=desc) (e9d66e9) will **decrease** coverage by `2.80%`.
> The diff coverage is `100.00%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12489/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12489 +/- ##
==========================================
- Coverage 66.12% 63.31% -2.81%
==========================================
Files 1015 486 -529
Lines 49554 29920 -19634
Branches 5079 0 -5079
==========================================
- Hits 32767 18945 -13822
+ Misses 16647 10975 -5672
+ Partials 140 0 -140
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.31% <100.00%> (-0.77%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/utils/date\_parser.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGF0ZV9wYXJzZXIucHk=) | `95.13% <100.00%> (+0.02%)` | :arrow_up: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/views.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.50% <0.00%> (-25.07%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.37% <0.00%> (-8.66%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/sql\_validators/base.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `79.38% <0.00%> (-6.45%)` | :arrow_down: |
| ... and [543 more](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=footer). Last update [e9d66e9...fb1a96a](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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] junlincc edited a comment on pull request #12489: fix: make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759335627
✅
thanks for the fix!!
**With lock**
<img width="1785" alt="Screen Shot 2021-01-13 at 1 50 14 AM" src="https://user-images.githubusercontent.com/67837651/104435891-c6b64000-5541-11eb-975c-14ff0d65eaf2.png">
**Without lock**
<img width="1767" alt="Screen Shot 2021-01-13 at 1 49 47 AM" src="https://user-images.githubusercontent.com/67837651/104435875-c0c05f00-5541-11eb-8c30-c266f6f6169e.png">
<img width="1763" alt="Screen Shot 2021-01-13 at 1 56 55 AM" src="https://user-images.githubusercontent.com/67837651/104436596-a76be280-5542-11eb-833a-b59ecf5aef4e.png">
<img width="1770" alt="Screen Shot 2021-01-13 at 1 56 46 AM" src="https://user-images.githubusercontent.com/67837651/104436600-a89d0f80-5542-11eb-9949-6c4018419e44.png">
----------------------------------------------------------------
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.
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] villebro commented on pull request #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759385906
> Looks like packrat cache comes with a lock for free: https://github.com/pyparsing/pyparsing/blob/f10fcf8c98ea07a63f8286288ce82f773e7e152e/pyparsing/core.py#L719
>
> So I'm 90% sure this will fix the bug... (with the additional benefit of making things a little faster).
I can confirm that this fixes the problem for me, too 👏 This looks like the perfect solution for this case. I think we can also re-enable the original memoization to get some additional perf improvement.
----------------------------------------------------------------
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.
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] mistercrunch commented on pull request #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759618261
This was easily one of the ugliest bug in Superset history: elusive, hard to reproduce, thread-safety-related, ...
Good work tracking it down and finding a viable fix for this one!
----------------------------------------------------------------
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.
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] villebro merged pull request #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
villebro merged pull request #12489:
URL: https://github.com/apache/superset/pull/12489
----------------------------------------------------------------
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.
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] junlincc commented on pull request #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759536991
countless thanks to you all for resolving this.🙏 @ktmud @villebro @zhaoyongjie
----------------------------------------------------------------
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.
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-io edited a comment on pull request #12489: fix(timepicker): make pyparsing thread safe
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12489:
URL: https://github.com/apache/superset/pull/12489#issuecomment-759358284
# [Codecov](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=h1) Report
> Merging [#12489](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=desc) (fb1a96a) into [master](https://codecov.io/gh/apache/superset/commit/e9d66e904f9dfdaab26ee3cc208fbcfddbed1df9?el=desc) (e9d66e9) will **decrease** coverage by `6.95%`.
> The diff coverage is `100.00%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12489/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12489 +/- ##
==========================================
- Coverage 66.12% 59.16% -6.96%
==========================================
Files 1015 959 -56
Lines 49554 46791 -2763
Branches 5079 4341 -738
==========================================
- Hits 32767 27684 -5083
- Misses 16647 19107 +2460
+ Partials 140 0 -140
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.99% <ø> (+5.49%)` | :arrow_up: |
| javascript | `?` | |
| python | `63.77% <100.00%> (-0.32%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/utils/date\_parser.py](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGF0ZV9wYXJzZXIucHk=) | `95.13% <100.00%> (+0.02%)` | :arrow_up: |
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...set-frontend/src/views/CRUD/alert/ExecutionLog.tsx](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYWxlcnQvRXhlY3V0aW9uTG9nLnRzeA==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [superset-frontend/src/components/IconTooltip.tsx](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvSWNvblRvb2x0aXAudHN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...end/src/SqlLab/components/ExploreResultsButton.jsx](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVSZXN1bHRzQnV0dG9uLmpzeA==) | `8.00% <0.00%> (-84.00%)` | :arrow_down: |
| ... and [415 more](https://codecov.io/gh/apache/superset/pull/12489/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=footer). Last update [e9d66e9...fb1a96a](https://codecov.io/gh/apache/superset/pull/12489?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
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