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/11/14 15:04:08 UTC
[GitHub] [superset] EugeneTorap opened a new pull request, #22118: Fix chart_slug is empty if chart name is non-ASCII
EugeneTorap opened a new pull request, #22118:
URL: https://github.com/apache/superset/pull/22118
If create a chart only with non-ASCII characters then we have '_<id>.yaml' filename and can't import this file because the filename with underscore.
This PR fix the issue just add a condition if chart_slug is empty then use only '<id>.yaml' filename
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #22118: fix: chart_slug is empty if chart name is non-ASCII
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #22118:
URL: https://github.com/apache/superset/pull/22118#issuecomment-1313934438
# [Codecov](https://codecov.io/gh/apache/superset/pull/22118?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 [#22118](https://codecov.io/gh/apache/superset/pull/22118?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9400147) into [master](https://codecov.io/gh/apache/superset/commit/5b67e0712d5d3c738abf3281e67755af741c5c01?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5b67e07) will **decrease** coverage by `1.43%`.
> The diff coverage is `66.66%`.
```diff
@@ Coverage Diff @@
## master #22118 +/- ##
==========================================
- Coverage 67.12% 65.69% -1.44%
==========================================
Files 1831 1831
Lines 69993 70003 +10
Branches 7570 7570
==========================================
- Hits 46983 45988 -995
- Misses 21045 22050 +1005
Partials 1965 1965
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `78.41% <66.66%> (+<0.01%)` | :arrow_up: |
| postgres | `78.46% <66.66%> (+<0.01%)` | :arrow_up: |
| presto | `?` | |
| python | `78.59% <66.66%> (-3.00%)` | :arrow_down: |
| sqlite | `76.93% <66.66%> (+<0.01%)` | :arrow_up: |
| unit | `?` | |
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/22118?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/charts/commands/export.py](https://codecov.io/gh/apache/superset/pull/22118/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-c3VwZXJzZXQvY2hhcnRzL2NvbW1hbmRzL2V4cG9ydC5weQ==) | `91.66% <66.66%> (-2.46%)` | :arrow_down: |
| [superset/tables/schemas.py](https://codecov.io/gh/apache/superset/pull/22118/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-c3VwZXJzZXQvdGFibGVzL3NjaGVtYXMucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset/columns/schemas.py](https://codecov.io/gh/apache/superset/pull/22118/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-c3VwZXJzZXQvY29sdW1ucy9zY2hlbWFzLnB5) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/22118/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-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [...set/advanced\_data\_type/plugins/internet\_address.py](https://codecov.io/gh/apache/superset/pull/22118/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-c3VwZXJzZXQvYWR2YW5jZWRfZGF0YV90eXBlL3BsdWdpbnMvaW50ZXJuZXRfYWRkcmVzcy5weQ==) | `16.32% <0.00%> (-79.60%)` | :arrow_down: |
| [superset/utils/pandas\_postprocessing/boxplot.py](https://codecov.io/gh/apache/superset/pull/22118/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-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nL2JveHBsb3QucHk=) | `20.51% <0.00%> (-79.49%)` | :arrow_down: |
| [superset/charts/post\_processing.py](https://codecov.io/gh/apache/superset/pull/22118/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-c3VwZXJzZXQvY2hhcnRzL3Bvc3RfcHJvY2Vzc2luZy5weQ==) | `11.76% <0.00%> (-77.95%)` | :arrow_down: |
| [...perset/advanced\_data\_type/plugins/internet\_port.py](https://codecov.io/gh/apache/superset/pull/22118/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-c3VwZXJzZXQvYWR2YW5jZWRfZGF0YV90eXBlL3BsdWdpbnMvaW50ZXJuZXRfcG9ydC5weQ==) | `18.75% <0.00%> (-77.09%)` | :arrow_down: |
| [superset/utils/pandas\_postprocessing/rolling.py](https://codecov.io/gh/apache/superset/pull/22118/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-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nL3JvbGxpbmcucHk=) | `21.87% <0.00%> (-68.75%)` | :arrow_down: |
| [...perset/utils/pandas\_postprocessing/contribution.py](https://codecov.io/gh/apache/superset/pull/22118/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-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nL2NvbnRyaWJ1dGlvbi5weQ==) | `34.61% <0.00%> (-65.39%)` | :arrow_down: |
| ... and [62 more](https://codecov.io/gh/apache/superset/pull/22118/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) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida merged pull request #22118: fix: slug is empty if filename is non-ASCII
Posted by GitBox <gi...@apache.org>.
betodealmeida merged PR #22118:
URL: https://github.com/apache/superset/pull/22118
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] EugeneTorap commented on pull request #22118: fix: slug is empty if filename is non-ASCII
Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #22118:
URL: https://github.com/apache/superset/pull/22118#issuecomment-1320306025
Hi @artemonsh! My PR fix the issue with Cyrillic characters.
If no, pls text me in superset slack. I have the same nickname there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] EugeneTorap commented on pull request #22118: fix: slug is empty if filename is non-ASCII
Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #22118:
URL: https://github.com/apache/superset/pull/22118#issuecomment-1320329984
@artemonsh I've read you suggested code. You only added Cyrillic support without others utf-8 characters like: hieroglyphs, emojis and so on. My PR supports import/export for all utf-8 characters.
--
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] artemonsh commented on pull request #22118: fix: slug is empty if filename is non-ASCII
Posted by GitBox <gi...@apache.org>.
artemonsh commented on PR #22118:
URL: https://github.com/apache/superset/pull/22118#issuecomment-1320268015
@EugeneTorap @villebro
Could you please take a look at this issue: https://github.com/apache/superset/issues/21657?
There is a quite annoying problem with cyrillic languages like Russian. In short, dashboards, charts and databases containing only cyrillic letters **cannot be imported into Superset**! The function Eugene provided, for instance, converts chart title "Мой график" ("My chart") into an empty string in python, leading to this type of filename: `_123.yaml`. And as the first character in the filename is underscore, the `is_valid_config` function returns `False` for this filename, which then prohibits to import the file, despite the notification in the bottom right corner saying that everything was imported successfully :/
Therefore, charts/dashboards/databases with these types of titles **cannot not be imported** into Superset. My suggestion is to expand werkzeug's `secure_filename` [function](https://tedboy.github.io/flask/_modules/werkzeug/utils.html#secure_filename) as follows:
```python
import unicodedata
def secure_filename(filename: str) -> str:
r"""Pass it a filename and it will return a secure version of it. This
filename can then safely be stored on a regular file system and passed
to :func:`os.path.join`.
On windows systems the function also makes sure that the file is not
named after one of the special device files.
The function also takes filenames containing cyrillic letters.
>>> secure_filename("My cool movie.mov")
'My_cool_movie.mov'
>>> secure_filename("../../../etc/passwd")
'etc_passwd'
>>> secure_filename('i contain cool \xfcml\xe4uts.txt')
'i_contain_cool_umlauts.txt'
>>> secure_filename('Мой красивый график.yaml')
'Мой_красивыи_график.yaml'
The function might return an empty filename. It's your responsibility
to ensure that the filename is unique and that you abort or
generate a random filename if the function returned an empty one.
.. versionadded:: 0.5
:param filename: the filename to secure
"""
# If the text contains cyrillic letters, ASCII encoding should not
# be used as it does not contain cyrillic letters
contains_cyrillic_letters = bool(re.search("[\u0400-\u04FF]", filename))
_windows_device_files = (
"CON",
"AUX",
"COM1",
"COM2",
"COM3",
"COM4",
"LPT1",
"LPT2",
"LPT3",
"PRN",
"NUL",
)
_filename_ascii_strip_re = re.compile(r"[^A-Za-z0-9_.-]")
_filename_strip_re = (
re.compile(r"[^A-Za-zа-яА-ЯёЁ0-9_.-]")
if contains_cyrillic_letters
else _filename_ascii_strip_re
)
filename = unicodedata.normalize("NFKD", filename)
if not contains_cyrillic_letters:
filename = filename.encode("ascii", "ignore").decode("ascii")
for sep in os.path.sep, os.path.altsep:
if sep:
filename = filename.replace(sep, " ")
filename = str(_filename_strip_re.sub("", "_".join(filename.split()))).strip("._")
# on nt a couple of special files are present in each folder. We
# have to ensure that the target file is not such a filename. In
# this case we prepend an underline
if (
os.name == "nt"
and filename
and filename.split(".")[0].upper() in _windows_device_files
):
filename = f"_{filename}"
return filename
```
Before:
```python
>>>print(secure_filename("Мой красивый график"))
>>>print(secure_filename("My beautiful график"))
>>>print(secure_filename("My beautiful chart"))
My_beautiful
My_beautiful_chart
```
After:
```python
>>>print(secure_filename("Мой красивый график"))
>>>print(secure_filename("My beautiful график"))
>>>print(secure_filename("My beautiful chart"))
Мои_красивыи_график
My_beautiful_график
My_beautiful_chart
```
--
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