Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove use of MetadataTableView in favor of utility functions #2927

Merged
merged 4 commits into from
Aug 10, 2018

Conversation

scottx611x
Copy link
Member

@scottx611x scottx611x commented Aug 2, 2018

MetadataTableView was not actually a View

@scottx611x scottx611x requested a review from hackdna August 2, 2018 15:56
@codecov
Copy link

codecov bot commented Aug 2, 2018

Codecov Report

Merging #2927 into develop will decrease coverage by 1.09%.
The diff coverage is 72.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop    #2927     +/-   ##
==========================================
- Coverage    60.46%   59.37%   -1.1%     
==========================================
  Files          435      435             
  Lines        28434    27290   -1144     
  Branches      1279     1273      -6     
==========================================
- Hits         17193    16203    -990     
+ Misses       11241    11087    -154
Impacted Files Coverage Δ
refinery/data_set_manager/views.py 60.13% <72.72%> (-5.9%) ⬇️
refinery/core/views.py 42.36% <0%> (-9.37%) ⬇️
...ce/js/dashboard/controllers/data-sets-card-ctrl.js 66.34% <0%> (-3.12%) ⬇️
refinery/core/models.py 76.08% <0%> (-0.89%) ⬇️
refinery/core/tests.py 99.73% <0%> (-0.15%) ⬇️
refinery/core/test_views.py 100% <0%> (ø) ⬆️
...nery/ui/source/js/commons/services/data-sets-v2.js 100% <0%> (ø) ⬆️
...rce/js/dashboard/services/data-set-card-factory.js 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a2c99a9...bf3f306. Read the comment docs.

@@ -169,72 +169,65 @@ def clean(self):
"Please provide either a file or a URL")


class MetaDataImportView(View):
def import_by_file(_file):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something more explicit like file_obj instead of _file?

error_msg,
e.errno,
e.filename,
e.strerror
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No extra line breaks necessary: error_msg, exc.errno, exc.filename, exc.strerror

temp_file_path = os.path.join(get_temp_dir(), _file.name)
try:
_handle_uploaded_file(_file, temp_file_path)
except IOError as e:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exc instead of e

@scottx611x scottx611x requested a review from hackdna August 9, 2018 20:05
@scottx611x scottx611x merged commit 17dd843 into develop Aug 10, 2018
@scottx611x scottx611x deleted the scottx611x/remove_metadatatableview branch August 10, 2018 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants