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

Fixes a small samba service regression #1496

Merged
merged 1 commit into from Nov 14, 2016

Conversation

MFlyer
Copy link
Member

@MFlyer MFlyer commented Oct 21, 2016

To @schakrava , sorry again, on dev env had no null field over ad config :/

Fixes #1495

Ready to merge before stable release
M.

@MFlyer
Copy link
Member Author

MFlyer commented Oct 23, 2016

To @schakrava, updates after solving with forum user: both this and #1492 can be safely merged, user issue not on Rockstor (uefi bios + r8169 driver card having nic up/down, moving to legacy bios solved it 😃 )

M.

@MFlyer MFlyer mentioned this pull request Nov 11, 2016
@tomtom13
Copy link
Contributor

Fixes #1515

@tomtom13
Copy link
Contributor

@schakrava can we expedite this pull ? I've tested a 3.8.15 without this fresh out of the box (and also after updating it it .1) and result is that one can NOT setup a successful samba session. Samba is running but one can not get to a share from windows machine either using guest or actual user account because master samba settings are not set correctly !!! (the workgroup bug block the rest defaults to be stored in samba config)

@MFlyer
Copy link
Member Author

MFlyer commented Nov 13, 2016

Hi @tomtom13 if you need it's just a 1 line code on src/rockstor/smart_manager/views/samba_service.py, having a reboot after changes should make it work on testing/stable env too! :)

To @schakrava : I fear this had to be merged before 3.8.15 (and I didn't remember to tell too!) because it's a blocking one

@tomtom13
Copy link
Contributor

@MFlyer it's not about me, I know how to patch my box :P it's about normal users, right now anybody installing rockstor will fail to install samba and for 90% home users it's a deal breaker if they can't have samba.

@MFlyer
Copy link
Member Author

MFlyer commented Nov 13, 2016

Hi @tomtom13 I know you can do that :P My #1459 probably was a damned premonition to this 😞

@schakrava @phillxnet & @sfranzen I believe we seriously have to handle "missing important patches":
my idea with #1459 is to have something requiring user interaction (have a form where users insert PR number then autopatch), but that forces user to manual intervention.
Suggestions? Can we have urgent PR merged and released on stable channel too?? not sure about that.
@schakrava can we add a new ad hoc yum repo just for emergency patching (so doing we won't have a new release like 3.8.15-1 or similar)

EDIT : https://forum.rockstor.com/t/cant-change-samba-workgroup/2332 here we come with the first one :(

@schakrava
Copy link
Member

@MFlyer and @tomtom13 . Thanks for this important discussion. I wish we merged this before 3.8.15, but it's too late now. There are a few takeaways from this experience. 1) Let's test our stuff thoroughly, and especially for backend, please add tests going forward. @phillxnet has added a bunch of them recently you can use as a reference to get up to speed on that. 2) I need to do a better job as part of our pre-stable release testing and you are right, a personal note about this regression would have helped, but I could've looked as well; shit happens. 3) Since the fix is so small, we can have it as "errata" for now. I am not in favour of pushing hot-fixes into stable. We can also put a console script that can pull an arbitrary gist and make hot-fixes easily applicable.

@schakrava
Copy link
Member

oh, finally @MFlyer thanks for the fix and sorry couldn't merge it fast enough :(

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.

None yet

3 participants