HUE-8505 [core] Close impala session on logout

Review Request #13273 - Created Aug. 17, 2018 and submitted

Chris Conner
hue
master
HUE-8505
hue
enricoberti, jgauthier, johan, ranade, romain, roohi, weixia, yingc
commit 29d46106b9bc0173df74b76b945a9ee80850d06d
Author: Chris Conner <cconner@cloudera.com>
Date:   Fri Aug 17 11:27:23 2018 -0400

    HUE-8505 [core] Close impala session on logout

:100644 100644 60b2b8b0a8... ca1fda05a4... M	desktop/core/src/desktop/auth/views.py

Tested that sessions are now closed when the logout button is clicked and when browser idle timeout is reached. I also confirmed that impala queries and session show up as closed from the Impala side and in Impala Queries list in CM.

  • 0
  • 0
  • 2
  • 0
  • 2
Description From Last Updated
  1. Nice!

    cf. comment, you could push and I tweak it?

  2. desktop/core/src/desktop/auth/views.py (Diff revision 1)
     
     

    Do you want me to tweak it so:
    - we only try when the user has Impala app?
    - we can directly call close_session (as create_session creates a new session if there is none existing)

    1. Yup, makes sense. I'll add the check for Impala app. Can we call close_session without a session ID? I'll give it a shot. open session was the only way I could find to get the session ID.

    2. Those should work IIRC:
      #1
      https://github.com/cloudera/hue/blob/master/desktop/core/src/desktop/models.py#L1664
      
      #2
      https://github.com/cloudera/hue/blob/master/desktop/libs/notebook/src/notebook/connectors/hiveserver2.py#L209
      if session_id is None:
        session = Session.objects.get_session(self.user, application=application)
        if session: 
          session_id = session.id
    3. Made all the changes.

  3. 
      
  1. Nice, one last comment, to keep it more modulated for later!

  2. desktop/core/src/desktop/auth/views.py (Diff revision 3)
     
     
     
     
     
     

    Could we move this into https://github.com/cloudera/hue/blob/master/desktop/libs/notebook/src/notebook/connectors/hiveserver2.py#L207

    so that it is more contained? (cleaner and avoid import errors when apps are disabled)

    If we don't specify "id" in
    session = {"type":"impala","id":session.id}, we can get it like you did.

    1. Were you mostly interested in these 2 lines?

      decoded_guid = session.get_handle().sessionId.guid
      session_id = "%x:%x" % struct.unpack(b"QQ", decoded_guid)
      

      Because they are not used by the line:

      session = {"type":"impala","id":session.id}
      

      That line is used to call close and must have "session.id" in it to call close successfully I think. The other 2 lines are just extras to get the good format session id to put in the LOG message:

      LOG.debug("Closing Impala session id %s on logout for user %s" % (session_id, username))
      

      So we wouldn't really be able to move the LOG message and those lines to hiveserver2.py unless we didn't care that the session close was from logout or we wanted to pass to close_session that we are closing due to logout.

      Or do you want to move the entire session finding to "close_session"? That would require passing the user to close_session. Thoughts?

    2. There is access to the user in https://github.com/cloudera/hue/blob/master/desktop/libs/notebook/src/notebook/connectors/hiveserver2.py#L215, e.g. self.user, so we could move almost everything but session = {"type":"impala"} there?

    3. Super, I'll give that a shot.

    4. Alright, made the changes:-).

  3. 
      
  1. Ship It!
  2. 
      
Review request changed

Status: Closed (submitted)

Loading...