hawkular / hawkular-client-ruby

Ruby client for Hawkular
http://hawkular.org/
Apache License 2.0
8 stars 28 forks source link

Connect operations client lazily #223

Closed cfcosta closed 7 years ago

cfcosta commented 7 years ago

This pull request fixes a 500 page that happens when you try to do a power operation when the middleware server is offline, by making the connection be created lazily when an operation is called.

The idea behind this is to allow operations code to catch errors when connecting to the server, because while errors from the operations can be catched.

As an example, here's this piece of code:

connection.operations(true).undeploy(deployment_data) do |on|
  on.success do |data|
    _log.debug "Success on websocket-operation #{data}"
    emit_middleware_notification(:mw_op_success, 'Undeploy', deployment_name, ems_ref, MiddlewareDeployment)
  end
  on.failure do |error|
    _log.error 'error callback was called, reason: ' + error.to_s
    emit_middleware_notification(:mw_op_failure, 'Undeploy', deployment_name, ems_ref, MiddlewareDeployment)
  end
end

If the error happens on #undeploy, then the failure clause will be called, but if the error happens on #operations(true), which spawns a new connection, it will be raised as normal.

As a bonus, this PR also removes the sleep code from the connection handler, which makes both the tests and real world usage much, much, much faster.

Before:

    hawkular-client-ruby git:(fix-operations-500-error) $ time RUBYOPT="-Ilib -W0 -r./bundle/bundler/setup.rb" rspec spec/integration/operations_spec.rb
    [Coveralls] Set up the SimpleCov formatter.
    [Coveralls] Using SimpleCov's default settings.
    Run options: exclude {:skip=>true}
    ...................

    Finished in 32.57 seconds (files took 0.69792 seconds to load)
    19 examples, 0 failures

After:

    hawkular-client-ruby git:(fix-operations-500-error) ✗ $ time RUBYOPT="-Ilib -W0 -r./bundle/bundler/setup.rb" rspec spec/integration/operations_spec.rb
    [Coveralls] Set up the SimpleCov formatter.
    [Coveralls] Using SimpleCov's default settings.
    Run options: exclude {:skip=>true}
    ...................

    Finished in 3.34 seconds (files took 0.74947 seconds to load)
    19 examples, 0 failures

Bugzilla Ticket: https://bugzilla.redhat.com/show_bug.cgi?id=1444940

cfcosta commented 7 years ago

@Jiri-Kremser @abonas can you take a look at this?

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.3%) to 95.669% when pulling f9c06ff6f53541933119cbe1a795445e5d6b6fe3 on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.006%) to 95.981% when pulling e8ddd5d05cd4c1511780b91cb9f1557c73f82640 on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

abonas commented 7 years ago

@israel-hdez please have a look

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.4%) to 95.533% when pulling 6dfbe4af0b92d1d31e497b7edf648ff164842338 on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.02%) to 95.997% when pulling 6dfbe4af0b92d1d31e497b7edf648ff164842338 on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.06%) to 96.033% when pulling 8413de5d285045da0965f82c6723fa0c70198aa1 on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.06%) to 96.033% when pulling 8413de5d285045da0965f82c6723fa0c70198aa1 on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-1.5%) to 94.49% when pulling f76e173112149d261aefd1a3ff94caf015b00246 on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.5%) to 95.513% when pulling f76e173112149d261aefd1a3ff94caf015b00246 on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.02%) to 95.96% when pulling 17a0d6eb2d548bce4c1ac431726ce8daae401bbc on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-1.5%) to 94.456% when pulling 02a89ef349ece19e980ff89236bee719074955d7 on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-1.5%) to 94.456% when pulling 02a89ef349ece19e980ff89236bee719074955d7 on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-1.5%) to 94.456% when pulling 02a89ef349ece19e980ff89236bee719074955d7 on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-1.5%) to 94.492% when pulling 931d3af7d1318fd49340f6509108e5fb61231f7d on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.05%) to 95.924% when pulling 889f7eefeddd728faf6d5e860f86c6254c5ec1ce on cfcosta:fix-operations-500-error into 6c14887936bfb86d6528d891a0d9becb5dd24efc on hawkular:master.

cfcosta commented 7 years ago

@israel-hdez @abonas did some changes, can you take a look at it again?

abonas commented 7 years ago

@pilhuhn please have a look

pilhuhn commented 7 years ago

I think that makes sense. I recall that we battled with getting the error callback right in the past for cases when credentials are wrong and similar.

israel-hdez commented 7 years ago

@cfcosta I have no more comments. Ready to merge?