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

Fix DataTables error on AFP shares view #1442

Merged
merged 2 commits into from
Sep 27, 2016

Conversation

sfranzen
Copy link
Contributor

This was caused by the table displayed in the absence of existing shares being classed data-table and stopped the view from loading fully. Partial fix of #1438, which also involves #813.

This was being caused by the "no shares" table having the data-table
class. Also fix the appearance of the same section on the sftp page to
look identical.
This fixes issues rockstor#813 and rockstor#1438, where adding a new share with disabled
service resulted in an error. The new behaviour is to automatically
start the service upon adding an AFP share.
@sfranzen
Copy link
Contributor Author

This now fixes the other issue and performs a systemctl reload-or-restart upon adding a new share. Perhaps this is a more intuitive UX for the other share types as well?

Copy link
Member

@schakrava schakrava left a comment

Choose a reason for hiding this comment

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

Thanks @sfranzen! Nicely done overall. I think we should move to python 3 compatible exception catching in other places, like you've done here. LGTM

@sfranzen
Copy link
Contributor Author

@schakrava Good idea. My IDE parser no longer accepts python 2, so it's marking these as invalid syntax. Looks like it's mostly "Exception, e" that it's complaining about though.

@priyaganti
Copy link
Contributor

Hey @sfranzen Thanks for finding out the issue and fixing it. Very good catch. I didn't notice it.

@schakrava schakrava merged commit 5503185 into rockstor:master Sep 27, 2016
@sfranzen sfranzen deleted the improve_afp_error branch September 29, 2016 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants