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 2020/01/29 17:11:13 UTC
[GitHub] [incubator-superset] dpgaspar opened a new pull request #9047: [csv
upload] Use python's named temp file
dpgaspar opened a new pull request #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047
### CATEGORY
- [ ] Bug Fix
- [X] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
Avoid file collisions and predictable filenames
### ADDITIONAL INFORMATION
- [ ] 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
### REVIEWERS
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request
#9047: [csv upload] Use python's named temp file
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047#discussion_r372941513
##########
File path: superset/views/database/views.py
##########
@@ -91,11 +91,14 @@ def form_post(self, form):
return redirect("/csvtodatabaseview/form")
csv_file = form.csv_file.data
- form.csv_file.data.filename = secure_filename(form.csv_file.data.filename)
+
csv_filename = form.csv_file.data.filename
- path = os.path.join(config["UPLOAD_FOLDER"], csv_filename)
+ extension = os.path.splitext(csv_filename)[1].lower()
Review comment:
Filename extension is checked on db_engine_spec, so I'm preserving it
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] craig-rueda commented on a change in pull
request #9047: [csv upload] Use python's named temp file
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047#discussion_r372519338
##########
File path: superset/views/database/views.py
##########
@@ -91,11 +91,14 @@ def form_post(self, form):
return redirect("/csvtodatabaseview/form")
csv_file = form.csv_file.data
- form.csv_file.data.filename = secure_filename(form.csv_file.data.filename)
+
csv_filename = form.csv_file.data.filename
- path = os.path.join(config["UPLOAD_FOLDER"], csv_filename)
+ extension = os.path.splitext(csv_filename)[1].lower()
+ path = tempfile.NamedTemporaryFile(
+ dir=app.config["UPLOAD_FOLDER"], suffix=extension
+ ).name
+ form.csv_file.data.filename = path
try:
- utils.ensure_path_exists(config["UPLOAD_FOLDER"])
csv_file.save(path)
Review comment:
I've seen an OOM issue with uploading "large" files - would be great to buffer these uploaded files to disk as opposed to storing them in mem
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #9047:
[csv upload] Use python's named temp file
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047#issuecomment-580313655
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9047?src=pr&el=h1) Report
> Merging [#9047](https://codecov.io/gh/apache/incubator-superset/pull/9047?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/6ac21dd938aba916cee3b38f73356f551d1e02f8?src=pr&el=desc) will **decrease** coverage by `0.29%`.
> The diff coverage is `79.71%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9047?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9047 +/- ##
=========================================
- Coverage 59.45% 59.15% -0.3%
=========================================
Files 369 370 +1
Lines 11747 11818 +71
Branches 2888 2900 +12
=========================================
+ Hits 6984 6991 +7
- Misses 4584 4648 +64
Partials 179 179
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9047?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...assets/src/components/ListView/TableCollection.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L1RhYmxlQ29sbGVjdGlvbi50c3g=) | `92.59% <ø> (ø)` | :arrow_up: |
| [superset/assets/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvYWN0aW9ucy9zcWxMYWIuanM=) | `61.96% <ø> (ø)` | :arrow_up: |
| [...rset/assets/src/dashboard/actions/sliceEntities.js](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvYWN0aW9ucy9zbGljZUVudGl0aWVzLmpz) | `9.67% <ø> (ø)` | :arrow_up: |
| [...perset/assets/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L0xpc3RWaWV3LnRzeA==) | `92.2% <100%> (ø)` | :arrow_up: |
| [superset/assets/src/SqlLab/constants.js](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29uc3RhbnRzLmpz) | `100% <100%> (ø)` | :arrow_up: |
| [...uperset/assets/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvY29tcG9uZW50cy9IZWFkZXIuanN4) | `42.06% <100%> (ø)` | :arrow_up: |
| [...rset/assets/src/components/ConfirmStatusChange.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0NvbmZpcm1TdGF0dXNDaGFuZ2UudHN4) | `100% <100%> (ø)` | :arrow_up: |
| [...uperset/assets/src/SqlLab/components/SqlEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29tcG9uZW50cy9TcWxFZGl0b3IuanN4) | `51.94% <100%> (ø)` | :arrow_up: |
| [superset/assets/src/chart/chartAction.js](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jaGFydC9jaGFydEFjdGlvbi5qcw==) | `47.94% <100%> (ø)` | :arrow_up: |
| [superset/assets/src/components/ListView/utils.ts](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L3V0aWxzLnRz) | `98.46% <100%> (ø)` | :arrow_up: |
| ... and [20 more](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9047?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/incubator-superset/pull/9047?src=pr&el=footer). Last update [6ac21dd...4eab54b](https://codecov.io/gh/apache/incubator-superset/pull/9047?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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] craig-rueda merged pull request #9047: [csv
upload] Use python's named temp file
Posted by GitBox <gi...@apache.org>.
craig-rueda merged pull request #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] craig-rueda commented on a change in pull
request #9047: [csv upload] Use python's named temp file
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047#discussion_r373120739
##########
File path: superset/views/database/views.py
##########
@@ -91,11 +91,14 @@ def form_post(self, form):
return redirect("/csvtodatabaseview/form")
csv_file = form.csv_file.data
- form.csv_file.data.filename = secure_filename(form.csv_file.data.filename)
+
csv_filename = form.csv_file.data.filename
- path = os.path.join(config["UPLOAD_FOLDER"], csv_filename)
+ extension = os.path.splitext(csv_filename)[1].lower()
Review comment:
Make sure this works with no file extension
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request
#9047: [csv upload] Use python's named temp file
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047#discussion_r373043185
##########
File path: superset/views/database/views.py
##########
@@ -91,11 +91,14 @@ def form_post(self, form):
return redirect("/csvtodatabaseview/form")
csv_file = form.csv_file.data
- form.csv_file.data.filename = secure_filename(form.csv_file.data.filename)
+
csv_filename = form.csv_file.data.filename
- path = os.path.join(config["UPLOAD_FOLDER"], csv_filename)
+ extension = os.path.splitext(csv_filename)[1].lower()
+ path = tempfile.NamedTemporaryFile(
+ dir=app.config["UPLOAD_FOLDER"], suffix=extension
+ ).name
+ form.csv_file.data.filename = path
try:
- utils.ensure_path_exists(config["UPLOAD_FOLDER"])
csv_file.save(path)
Review comment:
Did a stream upload with configurable chunks. Yet this may not address the OOM issue if it's related with the pandas dataframe csv loading
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request
#9047: [csv upload] Use python's named temp file
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047#discussion_r374774828
##########
File path: superset/views/database/views.py
##########
@@ -92,21 +106,23 @@ def form_post(self, form):
flash(message, "danger")
return redirect("/csvtodatabaseview/form")
- csv_file = form.csv_file.data
- form.csv_file.data.filename = secure_filename(form.csv_file.data.filename)
csv_filename = form.csv_file.data.filename
- path = os.path.join(config["UPLOAD_FOLDER"], csv_filename)
+ extension = os.path.splitext(csv_filename)[1].lower()
+ path = tempfile.NamedTemporaryFile(
+ dir=app.config["UPLOAD_FOLDER"], suffix=extension
+ ).name
Review comment:
Not sure if it makes any difference, but by default `NamedTemporaryFile` deletes the file after it is closed. I suppose it's ok to delete the file, too, but I think it should be safe to remove the `os.remove(path)`s below to keep the code lean.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #9047: [csv
upload] Use python's named temp file
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047#issuecomment-580313655
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9047?src=pr&el=h1) Report
> Merging [#9047](https://codecov.io/gh/apache/incubator-superset/pull/9047?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/6ac21dd938aba916cee3b38f73356f551d1e02f8?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `79.71%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9047?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9047 +/- ##
=======================================
Coverage 59.45% 59.45%
=======================================
Files 369 369
Lines 11747 11747
Branches 2888 2888
=======================================
Hits 6984 6984
Misses 4584 4584
Partials 179 179
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9047?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...assets/src/components/ListView/TableCollection.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L1RhYmxlQ29sbGVjdGlvbi50c3g=) | `92.59% <ø> (ø)` | :arrow_up: |
| [superset/assets/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvYWN0aW9ucy9zcWxMYWIuanM=) | `61.96% <ø> (ø)` | :arrow_up: |
| [...rset/assets/src/dashboard/actions/sliceEntities.js](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvYWN0aW9ucy9zbGljZUVudGl0aWVzLmpz) | `9.67% <ø> (ø)` | :arrow_up: |
| [...perset/assets/src/components/ListView/ListView.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L0xpc3RWaWV3LnRzeA==) | `92.2% <100%> (ø)` | :arrow_up: |
| [superset/assets/src/SqlLab/constants.js](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29uc3RhbnRzLmpz) | `100% <100%> (ø)` | :arrow_up: |
| [...uperset/assets/src/dashboard/components/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvY29tcG9uZW50cy9IZWFkZXIuanN4) | `42.06% <100%> (ø)` | :arrow_up: |
| [...rset/assets/src/components/ConfirmStatusChange.tsx](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0NvbmZpcm1TdGF0dXNDaGFuZ2UudHN4) | `100% <100%> (ø)` | :arrow_up: |
| [...uperset/assets/src/SqlLab/components/SqlEditor.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29tcG9uZW50cy9TcWxFZGl0b3IuanN4) | `51.94% <100%> (ø)` | :arrow_up: |
| [superset/assets/src/chart/chartAction.js](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jaGFydC9jaGFydEFjdGlvbi5qcw==) | `47.94% <100%> (ø)` | :arrow_up: |
| [superset/assets/src/components/ListView/utils.ts](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jb21wb25lbnRzL0xpc3RWaWV3L3V0aWxzLnRz) | `98.46% <100%> (ø)` | :arrow_up: |
| ... and [13 more](https://codecov.io/gh/apache/incubator-superset/pull/9047/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9047?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/incubator-superset/pull/9047?src=pr&el=footer). Last update [6ac21dd...245bb58](https://codecov.io/gh/apache/incubator-superset/pull/9047?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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] craig-rueda commented on a change in pull
request #9047: [csv upload] Use python's named temp file
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047#discussion_r372518595
##########
File path: superset/views/database/views.py
##########
@@ -91,11 +91,14 @@ def form_post(self, form):
return redirect("/csvtodatabaseview/form")
csv_file = form.csv_file.data
- form.csv_file.data.filename = secure_filename(form.csv_file.data.filename)
+
csv_filename = form.csv_file.data.filename
- path = os.path.join(config["UPLOAD_FOLDER"], csv_filename)
+ extension = os.path.splitext(csv_filename)[1].lower()
Review comment:
Probably best if we don't deal with extensions at all.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] robdiciuccio commented on issue #9047: [csv
upload] Use python's named temp file
Posted by GitBox <gi...@apache.org>.
robdiciuccio commented on issue #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047#issuecomment-582595501
Looks like this CSV upload logic is also overridden in `superset/db_engine_specs/hive.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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] dpgaspar commented on a change in pull request
#9047: [csv upload] Use python's named temp file
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047#discussion_r374132406
##########
File path: superset/views/database/views.py
##########
@@ -91,11 +91,14 @@ def form_post(self, form):
return redirect("/csvtodatabaseview/form")
csv_file = form.csv_file.data
- form.csv_file.data.filename = secure_filename(form.csv_file.data.filename)
+
csv_filename = form.csv_file.data.filename
- path = os.path.join(config["UPLOAD_FOLDER"], csv_filename)
+ extension = os.path.splitext(csv_filename)[1].lower()
Review comment:
Yes, tested and it's 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] craig-rueda commented on a change in pull
request #9047: [csv upload] Use python's named temp file
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #9047: [csv upload] Use python's named temp file
URL: https://github.com/apache/incubator-superset/pull/9047#discussion_r372518838
##########
File path: superset/views/database/views.py
##########
@@ -91,11 +91,14 @@ def form_post(self, form):
return redirect("/csvtodatabaseview/form")
csv_file = form.csv_file.data
- form.csv_file.data.filename = secure_filename(form.csv_file.data.filename)
+
csv_filename = form.csv_file.data.filename
- path = os.path.join(config["UPLOAD_FOLDER"], csv_filename)
+ extension = os.path.splitext(csv_filename)[1].lower()
+ path = tempfile.NamedTemporaryFile(
+ dir=app.config["UPLOAD_FOLDER"], suffix=extension
+ ).name
+ form.csv_file.data.filename = path
try:
- utils.ensure_path_exists(config["UPLOAD_FOLDER"])
csv_file.save(path)
Review comment:
Where does the file live before this call to save()? In mem?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org