#140 Add healthcheck API endpoint
Merged 5 years ago by jskladan. Opened 5 years ago by lholecek.
taskotron/ lholecek/resultsdb healthcheck  into  develop

Add healthcheck API endpoint
Lukas Holecek • 5 years ago  
@@ -804,6 +804,24 @@ 

      return jsonify(SERIALIZE(testcase)), 201

  

  

+ @api.route('/healthcheck', methods=['GET'])

+ def healthcheck():

+     """

+     Request handler for performing an application-level health check. This is

+     not part of the published API, it is intended for use by OpenShift or other

+     monitoring tools.

+ 

+     Returns a 200 response if the application is alive and able to serve requests.

+     """

+     try:

+         db.session.execute("SELECT 1 FROM result LIMIT 0").fetchall()

+     except Exception:

+         app.logger.exception('Healthcheck failed on DB query.')

+         return jsonify({"message": 'Unable to communicate with database'}), 503

+ 

+     return jsonify({"message": 'Health check OK'}), 200

+ 

+ 

  @api.route('', methods=['GET'])

  @api.route('/', methods=['GET'])

  def landing_page():

@@ -1012,3 +1012,18 @@ 

          data = json.loads(r.data)

          assert r.status_code == 300

          assert data['outcomes'] == ['PASSED', 'INFO', 'FAILED', 'NEEDS_INSPECTION', 'AMAZING']

+ 

+     def test_healthcheck_success(self):

+         r = self.app.get('/api/v2.0/healthcheck')

+         assert r.status_code == 200

+ 

+         data = json.loads(r.data)

+         assert data.get('message') == 'Health check OK'

+ 

+     def test_healthcheck_fail(self):

+         resultsdb.db.session.execute('DROP TABLE result')

+         r = self.app.get('/api/v2.0/healthcheck')

+         assert r.status_code == 503

+ 

+         data = json.loads(r.data)

+         assert data.get('message') == 'Unable to communicate with database'

JIRA: FACTORY-5428

Signed-off-by: Lukas Holecek hluk@email.cz

Looks good overall, but couple of nitpicks:

  • the whole API is JSON based ATM, so i'd rather see the responses be in that line (return jsonify({"message": MESSAGE}), HTTP_STATUS), just to keep things consistent
  • I don't think the HTTP response code on the 'failed' path is best suited for the job. I think HTTP 5xx (IMO 503 suits the best, but generic 500 would be fine) would be semantically better. Once again, this is not really a huge issue, obviously.

Once these are resolved, I'll be happy to merge the PR. Thanks!

rebased onto bd1f6b8436a0af8bd16450b3bcb09b3acc008463

5 years ago

rebased onto 9250ae9

5 years ago

OK, I've changed the healthcheck response to JSON and HTTP status for failure to 503.

Looks OK to me, thanks. Not the biggest fan of general except Exception but in this specific case, I think it's fine.

If @lucarval or @lslebodn won't give any negative feedback, I'll merge the PR tomorrow.

Thanks again @lholecek

I have zero experience with resultsdb internals.
I am not sure whether I am the best for review. But code LGTM :-)

@lslebodn Oops, sorry, I mistakenly pinged you for review instead of jskladan. :)

Hehe, things happen. Merging...

Pull-Request has been merged by jskladan

5 years ago