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

failure to submit on modal service dialogs #1537

Closed
phillxnet opened this Issue Nov 19, 2016 · 18 comments

Comments

Projects
None yet
3 participants
@phillxnet
Member

phillxnet commented Nov 19, 2016

Rockstor testing release 3.8.15-7 unable to submit service config (spanner) via modal dialogs.

rock-on config:
Unknown internal error doing a POST to /api/sm/services/docker/config

nut: (with existing and unchanged prior working config)
Unknown internal error doing a POST to /api/sm/services/nut/config

samba: MYGROUP with empty global section:
Unknown internal error doing a POST to /api/sm/services/smb/config

etc...

Web browser console point to:
POST https://delll.lan/api/sm/services/nut/config 500 (INTERNAL SERVER ERROR)

                var jqxhr = $.ajax({
                    url: "/api/sm/services/" + _this.serviceName + "/config",
                    type: "POST",
                    contentType: 'application/json',
                    dataType: "html",
                    data: data
                });

@phillxnet phillxnet changed the title from failure to submit on model service dialogs to failure to submit on modal service dialogs Nov 19, 2016

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 19, 2016

Member

Confirmed on two systems upgraded from 3.8.15-5 to 3.8.15-7

confirmed working spanner submit (non modal) prior to upgrade.

Member

phillxnet commented Nov 19, 2016

Confirmed on two systems upgraded from 3.8.15-5 to 3.8.15-7

confirmed working spanner submit (non modal) prior to upgrade.

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 19, 2016

Member

@MFlyer can you re-produce this, I know there were recent changes in this area re your comments and quoted code snippet in #1529. But I'm just not that up on this side of things yet.

Issue replicated in Firefox as well as Chrome.

Member

phillxnet commented Nov 19, 2016

@MFlyer can you re-produce this, I know there were recent changes in this area re your comments and quoted code snippet in #1529. But I'm just not that up on this side of things yet.

Issue replicated in Firefox as well as Chrome.

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Nov 19, 2016

Member

Hi @phillxnet ,
was having some on the fly tests while cooking rice :D
Actually on a DEV ENV I can configure services and get errors on NUT while having switch on/off
Houston, we've had a problem.

Failed to start NUT-UPS service due to a system error. Check the service is configured correctly via it's spanner icon.
 Traceback (most recent call last):
  File "/opt/build/src/rockstor/smart_manager/views/nut_service.py", line 41, in post
    self._switch_nut(command, self._get_config(service))
  File "/opt/build/src/rockstor/smart_manager/views/nut_service.py", line 64, in _switch_nut
    systemctl('nut-server', 'start')
  File "/opt/build/src/rockstor/system/services.py", line 64, in systemctl
    return run_command([SYSTEMCTL_BIN, switch, service_name])
  File "/opt/build/src/rockstor/system/osi.py", line 109, in run_command
    raise CommandException(cmd, out, err, rc)
CommandException: Error running a command. cmd = ['/usr/bin/systemctl', 'start', 'nut-server']. rc = 1. stdout = ['']. stderr = ["A dependency job for nut-server.service failed. See 'journalctl -xe' for details.", '']

No errors on Rock-on on/off, samba etc.
Can you have this test?:
0e74912 <- move back to json
have same checks on a dev env

Thanks
M.
Back to rice!!!

Member

MFlyer commented Nov 19, 2016

Hi @phillxnet ,
was having some on the fly tests while cooking rice :D
Actually on a DEV ENV I can configure services and get errors on NUT while having switch on/off
Houston, we've had a problem.

Failed to start NUT-UPS service due to a system error. Check the service is configured correctly via it's spanner icon.
 Traceback (most recent call last):
  File "/opt/build/src/rockstor/smart_manager/views/nut_service.py", line 41, in post
    self._switch_nut(command, self._get_config(service))
  File "/opt/build/src/rockstor/smart_manager/views/nut_service.py", line 64, in _switch_nut
    systemctl('nut-server', 'start')
  File "/opt/build/src/rockstor/system/services.py", line 64, in systemctl
    return run_command([SYSTEMCTL_BIN, switch, service_name])
  File "/opt/build/src/rockstor/system/osi.py", line 109, in run_command
    raise CommandException(cmd, out, err, rc)
CommandException: Error running a command. cmd = ['/usr/bin/systemctl', 'start', 'nut-server']. rc = 1. stdout = ['']. stderr = ["A dependency job for nut-server.service failed. See 'journalctl -xe' for details.", '']

No errors on Rock-on on/off, samba etc.
Can you have this test?:
0e74912 <- move back to json
have same checks on a dev env

Thanks
M.
Back to rice!!!

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 19, 2016

Member

@MFlyer That is as stated that your nut config is problematic. This occurs on all services tested so far.
Not during on off but during config submit.
Just go into ie samba, enter MYGROUP with empty global section:
press submit button: error
all services seem affected similarly. (Nut more tricky to get legit config).
Also note rock-ons config example given.

This is with testing release not DEV environment, hence the concern.

Will report back with your suggestion shortly. Enjoy the rice and thanks for pointer.

Member

phillxnet commented Nov 19, 2016

@MFlyer That is as stated that your nut config is problematic. This occurs on all services tested so far.
Not during on off but during config submit.
Just go into ie samba, enter MYGROUP with empty global section:
press submit button: error
all services seem affected similarly. (Nut more tricky to get legit config).
Also note rock-ons config example given.

This is with testing release not DEV environment, hence the concern.

Will report back with your suggestion shortly. Enjoy the rice and thanks for pointer.

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Nov 19, 2016

Member

hi @tastyratz,
call you because you've just closed other issue about this with #1528 (comment).
Can you confirm it's running on latest Rockstor 3.8.15-7?

Member

MFlyer commented Nov 19, 2016

hi @tastyratz,
call you because you've just closed other issue about this with #1528 (comment).
Can you confirm it's running on latest Rockstor 3.8.15-7?

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 19, 2016

Member

Clever @MFlyer : Yes changing the type back to json (from html)
running the collectstatic line
and we have successful submit button on our new modal service config dialogs.
Tested nut, samba, SMART.

Can I leave this one with you as I'm unsure of the consequences elsewhere on this change and need to get disk role tied down a tad more.

Member

phillxnet commented Nov 19, 2016

Clever @MFlyer : Yes changing the type back to json (from html)
running the collectstatic line
and we have successful submit button on our new modal service config dialogs.
Tested nut, samba, SMART.

Can I leave this one with you as I'm unsure of the consequences elsewhere on this change and need to get disk role tied down a tad more.

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 19, 2016

Member

@MFlyer I saw issue #1528 but that seemed to be related to the modal config dialogs not appearing in the first place.

Member

phillxnet commented Nov 19, 2016

@MFlyer I saw issue #1528 but that seemed to be related to the modal config dialogs not appearing in the first place.

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Nov 19, 2016

Member

Hi @phillxnet thanks confirming that :)
thant json to html was made to solve a firefox glitch found by @schakrava but it seems we have different reactions on dev env and testing/stable:
on dev env Django Rest Framework responses with html
on testing/stable with json

@schakrava any idea about this?

To @phillxnet on the other side i get nut not working on dev env (that error reported) : don't care about this, had some random configs, all was saved, but nut not starting. Not related to modal

Member

MFlyer commented Nov 19, 2016

Hi @phillxnet thanks confirming that :)
thant json to html was made to solve a firefox glitch found by @schakrava but it seems we have different reactions on dev env and testing/stable:
on dev env Django Rest Framework responses with html
on testing/stable with json

@schakrava any idea about this?

To @phillxnet on the other side i get nut not working on dev env (that error reported) : don't care about this, had some random configs, all was saved, but nut not starting. Not related to modal

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 19, 2016

Member

@MFlyer nut error red herring as config is checked to be legit and error states non legit. Will only start if legit config which is tricky to get without a ups or nut server. Although there is a dummy driver setup option but a bit off topic for this issue.

All examples tried work here after change back to json on second testing channel (rpm install) machine as well:
ie nut, samba, SMART. all with legit config in this case.

Firefox test on second machine required browser refresh on services page and it also then accepting submit button without error, (but FF dev console displays "no element found" after each submit button (orange) ).

Sorry to have disturbed your rice and thanks for the quick response, potential resolution, on this one.

Member

phillxnet commented Nov 19, 2016

@MFlyer nut error red herring as config is checked to be legit and error states non legit. Will only start if legit config which is tricky to get without a ups or nut server. Although there is a dummy driver setup option but a bit off topic for this issue.

All examples tried work here after change back to json on second testing channel (rpm install) machine as well:
ie nut, samba, SMART. all with legit config in this case.

Firefox test on second machine required browser refresh on services page and it also then accepting submit button without error, (but FF dev console displays "no element found" after each submit button (orange) ).

Sorry to have disturbed your rice and thanks for the quick response, potential resolution, on this one.

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Nov 19, 2016

Member

@phillxnet your last test is once again useful (you anticipated my next question about Firefox glitch):

no element found on Firefox is the glitch by @schakrava , so having it again help us testing:

  • Firefox is more accurate than Chrome/Chromium
  • having dataType : "json" and getting html doesn't alert Chrome/Chromium, but raises a warning (not an error) on meticulous Firefox
  • seem both dev env and testing/stable branches have html response, but expliciting it on ajax in testing/stable branches fails, not on dev env

Going deeper into this (we can fix it via a rollback to json instead of html, having always that glitch on firefox), probably asking you soon to have another buildstatic on testing branch :)

Member

MFlyer commented Nov 19, 2016

@phillxnet your last test is once again useful (you anticipated my next question about Firefox glitch):

no element found on Firefox is the glitch by @schakrava , so having it again help us testing:

  • Firefox is more accurate than Chrome/Chromium
  • having dataType : "json" and getting html doesn't alert Chrome/Chromium, but raises a warning (not an error) on meticulous Firefox
  • seem both dev env and testing/stable branches have html response, but expliciting it on ajax in testing/stable branches fails, not on dev env

Going deeper into this (we can fix it via a rollback to json instead of html, having always that glitch on firefox), probably asking you soon to have another buildstatic on testing branch :)

MFlyer added a commit to MFlyer/rockstor-core that referenced this issue Nov 19, 2016

#1537 - Rollback to json datatype, html blocking submits on testing/s…
…table branches

Signed-off-by: Mirko Arena <mirko.arena@gmail.com>
@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Nov 19, 2016

Member

Hi @schakrava ,
while having other tests to fix that glitch too, we can merge #1538 to grant users full working config pages.
Obviously waiting your speech about this testing/stable branches vs dev env diffs on DRF :)

M.

Member

MFlyer commented Nov 19, 2016

Hi @schakrava ,
while having other tests to fix that glitch too, we can merge #1538 to grant users full working config pages.
Obviously waiting your speech about this testing/stable branches vs dev env diffs on DRF :)

M.

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Nov 19, 2016

Member

Hi all finally got it! 😃

2b81748 is ok and we can merge it

Firefox glitch @schakrava
Using samba service to have an example (https://github.com/rockstor/rockstor-core/blob/master/src/rockstor/smart_manager/views/samba_service.py#L109), let's modify our response to

return Response('{}')

This makes Firefox happy, because on our ajax request we "promise" Firefox a json will be returned, while Response() returns an emtpy line
...that's why I love and hate Firefox, but I can't totally blame it :)

Going to have all POSTS responses serving an empty object to make FF feel happy :)
@phillxnet can you confirm editing only that samba_service.py + one reboot and submit samba config? thx
M.

Member

MFlyer commented Nov 19, 2016

Hi all finally got it! 😃

2b81748 is ok and we can merge it

Firefox glitch @schakrava
Using samba service to have an example (https://github.com/rockstor/rockstor-core/blob/master/src/rockstor/smart_manager/views/samba_service.py#L109), let's modify our response to

return Response('{}')

This makes Firefox happy, because on our ajax request we "promise" Firefox a json will be returned, while Response() returns an emtpy line
...that's why I love and hate Firefox, but I can't totally blame it :)

Going to have all POSTS responses serving an empty object to make FF feel happy :)
@phillxnet can you confirm editing only that samba_service.py + one reboot and submit samba config? thx
M.

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 19, 2016

Member

@MFlyer OK, I'll try and get to this tomorrow. Well done.

Member

phillxnet commented Nov 19, 2016

@MFlyer OK, I'll try and get to this tomorrow. Well done.

schakrava added a commit that referenced this issue Nov 20, 2016

Merge pull request #1538 from MFlyer/issue#1537_dataType_modal_submit
#1537 - Rollback to json datatype, html blocking submits on testing/s…
@schakrava

This comment has been minimized.

Show comment
Hide comment
@schakrava

schakrava Nov 20, 2016

Member

@MFlyer nice work investigating the "glitch". I think we are breaking REST conventions by the way we are implementing commands(config, start and stop) via POST. I am ok with your fix of sending empty object in the response. Another half-solution is to respond with the Service object since, at least in case of config, it is the resource that potentially gets modified. Either way we don't make REST gods happy enough. I say proceed with empty object response as it works, but if it interests you, think more about how we can improve the design to make it more RESTful.

Member

schakrava commented Nov 20, 2016

@MFlyer nice work investigating the "glitch". I think we are breaking REST conventions by the way we are implementing commands(config, start and stop) via POST. I am ok with your fix of sending empty object in the response. Another half-solution is to respond with the Service object since, at least in case of config, it is the resource that potentially gets modified. Either way we don't make REST gods happy enough. I say proceed with empty object response as it works, but if it interests you, think more about how we can improve the design to make it more RESTful.

@schakrava

This comment has been minimized.

Show comment
Hide comment
@schakrava

schakrava Nov 20, 2016

Member

And sorry I am unable to respond fast enough. But happy to catch up on the conversation and thanks @phillxnet for your diligence.

Member

schakrava commented Nov 20, 2016

And sorry I am unable to respond fast enough. But happy to catch up on the conversation and thanks @phillxnet for your diligence.

@schakrava

This comment has been minimized.

Show comment
Hide comment
@schakrava

schakrava Nov 20, 2016

Member

Fixed by #1538

Member

schakrava commented Nov 20, 2016

Fixed by #1538

@phillxnet

This comment has been minimized.

Show comment
Hide comment
@phillxnet

phillxnet Nov 20, 2016

Member

@schakrava Cheers.
Closing as Fixed by #1538
Thanks to @MFlyer

Member

phillxnet commented Nov 20, 2016

@schakrava Cheers.
Closing as Fixed by #1538
Thanks to @MFlyer

@phillxnet phillxnet closed this Nov 20, 2016

@MFlyer

This comment has been minimized.

Show comment
Hide comment
@MFlyer

MFlyer Nov 20, 2016

Member

@schakrava will think about possible alternative responses (actually don't find it useful to have something like {'service': 'name', 'status':'configured/started/stopped'} or return service itself, this gives us info we already have via data_collector/responses on errors)

Member

MFlyer commented Nov 20, 2016

@schakrava will think about possible alternative responses (actually don't find it useful to have something like {'service': 'name', 'status':'configured/started/stopped'} or return service itself, this gives us info we already have via data_collector/responses on errors)

@schakrava schakrava added the bug label Nov 20, 2016

@schakrava schakrava added this to the Point Bonita milestone Nov 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment