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/25 08:29:30 UTC

[GitHub] [superset] iercan opened a new pull request #12728: Erroneous uuid generation for mysql fixed.

iercan opened a new pull request #12728:
URL: https://github.com/apache/superset/pull/12728


   ### SUMMARY
   This fix related with #12202. Mysql uuid generation is erroneous and should not be used. 
   
   ### 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 -->
   - [X] Has associated issue: #12202 
   - [ ] 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] iercan commented on pull request #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
iercan commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-766982884


   We have mysql 5.7.32. Not sure it is relevant but we were using amancevice/superset:0.38 docker image and moved to apache/superset:1.0.0.


----------------------------------------------------------------
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 #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-766976472


   It's weird because Airbnb uses MySQL and the migration was successful. Maybe `uuid()` behave differently in different versions?
   
   I'll do some more testing and see if there is an efficient fix.


----------------------------------------------------------------
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] juneauwang commented on pull request #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
juneauwang commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-769695584


   Hi guys, 
   Thanks a lot for quick response! It's incredible. 
   Finally I found that it's my bad that I used wrong secret key in config. 
   Thanks again! 
   
   
   


----------------------------------------------------------------
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] iercan commented on a change in pull request #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
iercan commented on a change in pull request #12728:
URL: https://github.com/apache/superset/pull/12728#discussion_r565062535



##########
File path: superset/migrations/versions/b56500de1855_add_uuid_column_to_import_mixin.py
##########
@@ -81,7 +81,8 @@ class ImportMixin:
 
 # Add uuids directly using built-in SQL uuid function
 add_uuids_by_dialect = {
-    MySQLDialect: """UPDATE %s SET uuid = UNHEX(REPLACE(uuid(), "-", ""));""",
+# TODO uuid from mysql aren't working as expected. that's why shouldn't be used for now. 
+#    MySQLDialect: """UPDATE %s SET uuid = UNHEX(REPLACE(uuid(), "-", ""));""",

Review comment:
       This solution worked. I will create another PR




----------------------------------------------------------------
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 #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-769636802


   > Hi guys,
   > Also ran into this issue and tried to implement before 1.0.1
   > I manually implemented
   > `MySQLDialect: """UPDATE %s SET uuid = U UNHEX(REPLACE(CONVERT(UUID() using utf8mb4), '-', ''));"""`
   > but runs into
   > 
   > ```
   > Traceback (most recent call last):
   >   File "/srv/python3/lib/python3.8/site-packages/sqlalchemy_utils/types/encrypted/encrypted_type.py", line 126, in decrypt
   >     decrypted = decrypted.decode('utf-8')
   > UnicodeDecodeError: 'utf-8' codec can't decode byte 0xc2 in position 0: invalid continuation byte
   > ```
   > 
   > My versions:
   > Python 3.8.2
   > mysql Ver 14.14 Distrib 5.7.23-23, for Linux (x86_64) using 6.0
   > SQLAlchemy 1.3.20
   > SQLAlchemy-Utils 0.36.8
   > apache-superset 1.0.0
   
   Thanks for reporting @juneauwang; can you check what character set your metadata tables have? 


----------------------------------------------------------------
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 #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-766689163


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12728?src=pr&el=h1) Report
   > Merging [#12728](https://codecov.io/gh/apache/superset/pull/12728?src=pr&el=desc) (c1cfca9) into [master](https://codecov.io/gh/apache/superset/commit/307e3a9f653cc0f9c9474a17d14e9371211414df?el=desc) (307e3a9) will **decrease** coverage by `3.61%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12728/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12728?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12728      +/-   ##
   ==========================================
   - Coverage   66.83%   63.21%   -3.62%     
   ==========================================
     Files        1022      488     -534     
     Lines       49992    30102   -19890     
     Branches     4892        0    -4892     
   ==========================================
   - Hits        33410    19028   -14382     
   + Misses      16457    11074    -5383     
   + Partials      125        0     -125     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.21% <ø> (-0.79%)` | :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/12728?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ns/b56500de1855\_add\_uuid\_column\_to\_import\_mixin.py](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy9iNTY1MDBkZTE4NTVfYWRkX3V1aWRfY29sdW1uX3RvX2ltcG9ydF9taXhpbi5weQ==) | `0.00% <ø> (ø)` | |
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `54.61% <0.00%> (-29.24%)` | :arrow_down: |
   | [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12728/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/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `84.31% <0.00%> (-6.28%)` | :arrow_down: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
   | [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12728/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/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `96.42% <0.00%> (-3.58%)` | :arrow_down: |
   | ... and [546 more](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12728?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/12728?src=pr&el=footer). Last update [307e3a9...c1cfca9](https://codecov.io/gh/apache/superset/pull/12728?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] iercan commented on pull request #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
iercan commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-766982884


   We have mysql 5.7.32. Not sure it is relevant but we were using amancevice/superset:0.38 docker image and moved to apache/superset:1.0.0.


----------------------------------------------------------------
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 #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-769636778


   @juneauwang can you paste the full call stack if available? I wonder which process called that line in `encrypted_type.py`.


----------------------------------------------------------------
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] iercan commented on a change in pull request #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
iercan commented on a change in pull request #12728:
URL: https://github.com/apache/superset/pull/12728#discussion_r565062535



##########
File path: superset/migrations/versions/b56500de1855_add_uuid_column_to_import_mixin.py
##########
@@ -81,7 +81,8 @@ class ImportMixin:
 
 # Add uuids directly using built-in SQL uuid function
 add_uuids_by_dialect = {
-    MySQLDialect: """UPDATE %s SET uuid = UNHEX(REPLACE(uuid(), "-", ""));""",
+# TODO uuid from mysql aren't working as expected. that's why shouldn't be used for now. 
+#    MySQLDialect: """UPDATE %s SET uuid = UNHEX(REPLACE(uuid(), "-", ""));""",

Review comment:
       This solution worked. I will create another PR
   
   Edit: It worked with a little change. 
   `MySQLDialect: """UPDATE %s SET uuid = UNHEX(REPLACE(CONVERT(UUID() using utf8mb4), '-', ''));""",`




----------------------------------------------------------------
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] iercan edited a comment on pull request #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
iercan edited a comment on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-769639981


   @juneauwang @villebro  It should be like this exactly. There was a little error on Beto's solution.  Sorry I forgot to mention. 
   ```
   MySQLDialect: """UPDATE %s SET uuid = UNHEX(REPLACE(CONVERT(UUID() using utf8mb4), '-', ''));""",
   ``` 
   
   This is the merged 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] iercan closed pull request #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
iercan closed pull request #12728:
URL: https://github.com/apache/superset/pull/12728


   


----------------------------------------------------------------
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 #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-766689163


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12728?src=pr&el=h1) Report
   > Merging [#12728](https://codecov.io/gh/apache/superset/pull/12728?src=pr&el=desc) (c1cfca9) into [master](https://codecov.io/gh/apache/superset/commit/307e3a9f653cc0f9c9474a17d14e9371211414df?el=desc) (307e3a9) will **decrease** coverage by `3.61%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12728/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12728?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12728      +/-   ##
   ==========================================
   - Coverage   66.83%   63.21%   -3.62%     
   ==========================================
     Files        1022      488     -534     
     Lines       49992    30102   -19890     
     Branches     4892        0    -4892     
   ==========================================
   - Hits        33410    19028   -14382     
   + Misses      16457    11074    -5383     
   + Partials      125        0     -125     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.21% <ø> (-0.79%)` | :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/12728?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ns/b56500de1855\_add\_uuid\_column\_to\_import\_mixin.py](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy9iNTY1MDBkZTE4NTVfYWRkX3V1aWRfY29sdW1uX3RvX2ltcG9ydF9taXhpbi5weQ==) | `0.00% <ø> (ø)` | |
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `54.61% <0.00%> (-29.24%)` | :arrow_down: |
   | [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12728/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/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `84.31% <0.00%> (-6.28%)` | :arrow_down: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
   | [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12728/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/12728/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `96.42% <0.00%> (-3.58%)` | :arrow_down: |
   | ... and [546 more](https://codecov.io/gh/apache/superset/pull/12728/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12728?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/12728?src=pr&el=footer). Last update [307e3a9...c1cfca9](https://codecov.io/gh/apache/superset/pull/12728?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] betodealmeida commented on a change in pull request #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #12728:
URL: https://github.com/apache/superset/pull/12728#discussion_r564942676



##########
File path: superset/migrations/versions/b56500de1855_add_uuid_column_to_import_mixin.py
##########
@@ -81,7 +81,8 @@ class ImportMixin:
 
 # Add uuids directly using built-in SQL uuid function
 add_uuids_by_dialect = {
-    MySQLDialect: """UPDATE %s SET uuid = UNHEX(REPLACE(uuid(), "-", ""));""",
+# TODO uuid from mysql aren't working as expected. that's why shouldn't be used for now. 
+#    MySQLDialect: """UPDATE %s SET uuid = UNHEX(REPLACE(uuid(), "-", ""));""",

Review comment:
       @iercan can you try changing it to:
   
   ```python
   MySQLDialect: """UPDATE %s SET uuid = U UNHEX(REPLACE(CONVERT(UUID() using utf8mb4), '-', ''));"""
   ```
   
   Based on https://dba.stackexchange.com/a/278540




----------------------------------------------------------------
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] betodealmeida commented on pull request #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
betodealmeida commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-767927170


   > Apparently `UUID()` on MySQL is UUID1: https://dev.mysql.com/doc/refman/5.7/en/miscellaneous-functions.html#function_uuid 🙁 As we're relying on UUIDs going forward, I don't think we can disable this step, so we actually have to come up with a solution that populates UUIDs for these rows.
   
   @villebro UUID1 should be fine for our needs, it doesn't have to be UUID4. And note that @iercan's solution just removes the optimization, they UUIDs are still being populated for MySQL using Python's `uuid4()`.


----------------------------------------------------------------
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] iercan commented on pull request #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
iercan commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-767328359


   I will try to solve it by using mysql and send another PR later


----------------------------------------------------------------
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] juneauwang commented on pull request #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
juneauwang commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-769603234


   Hi guys, 
   Also ran into this issue and tried to implement before 1.0.1
   I manually implemented
   `MySQLDialect: """UPDATE %s SET uuid = U UNHEX(REPLACE(CONVERT(UUID() using utf8mb4), '-', ''));"""`
   but runs into 
   ```
   Traceback (most recent call last):
     File "/srv/python3/lib/python3.8/site-packages/sqlalchemy_utils/types/encrypted/encrypted_type.py", line 126, in decrypt
       decrypted = decrypted.decode('utf-8')
   UnicodeDecodeError: 'utf-8' codec can't decode byte 0xc2 in position 0: invalid continuation byte
   ```
   
   My versions: 
   Python 3.8.2
   mysql  Ver 14.14 Distrib 5.7.23-23, for Linux (x86_64) using  6.0
   SQLAlchemy             1.3.20
   SQLAlchemy-Utils       0.36.8
   apache-superset        1.0.0


----------------------------------------------------------------
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 #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-766976472


   It's weird because Airbnb uses MySQL and the migration was successful. Maybe `uuid()` behave differently in different versions?
   
   I'll do some more testing and see if there is an efficient fix.


----------------------------------------------------------------
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] iercan commented on pull request #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
iercan commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-769639981


   @juneauwang @villebro  It should be like this exactly. There was a little error on Beto's solution.  Sorry I forgot to mention. 
   ```
   MySQLDialect: """UPDATE %s SET uuid = UNHEX(REPLACE(CONVERT(UUID() using utf8mb4), '-', ''));""",
   ``` 


----------------------------------------------------------------
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 #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-769645637


   Oh nice, thanks for that @iercan , apparently just a typo then 👍 


----------------------------------------------------------------
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 #12728: fix: Erroneous uuid generation for mysql fixed.

Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #12728:
URL: https://github.com/apache/superset/pull/12728#issuecomment-768081624


   > > Apparently `UUID()` on MySQL is UUID1: https://dev.mysql.com/doc/refman/5.7/en/miscellaneous-functions.html#function_uuid 🙁 As we're relying on UUIDs going forward, I don't think we can disable this step, so we actually have to come up with a solution that populates UUIDs for these rows.
   > 
   > And note that @iercan's solution just removes the optimization, they UUIDs are still being populated for MySQL using Python's `uuid4()`.
   
   Oh, thanks for correcting, I had completely missed that part! *grabs my coat*


----------------------------------------------------------------
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