Bug 425 - Commandline tool for listing sessions
Summary: Commandline tool for listing sessions
Status: CLOSED FIXED
Alias: None
Product: ThinLinc
Classification: Unclassified
Component: Other (show other bugs)
Version: trunk
Hardware: PC Linux
: P2 Enhancement
Target Milestone: 4.15.0
Assignee: William Sjöblom
URL:
Keywords: relnotes, tobfa_tester
Depends on: 7860
Blocks: 3707
  Show dependency treegraph
 
Reported: 2003-12-18 16:10 CET by Erik Forsberg
Modified: 2023-03-30 09:43 CEST (History)
5 users (show)

See Also:
Acceptance Criteria:
* It should be possible to list: - All sessions - All sessions for a user - All sessions on an agent - All sessions in a cluster * The listed information should contain: - When the session was created - Agent - Username - Display number - Status - The user should be able to tell the connection status of a session or if a session is unreachable * The list output should by default be sorted on username


Attachments

Description Erik Forsberg cendio 2003-12-18 16:10:07 CET
There is no commandline tool for listing active sessions on the server. You
either have to check the vsmserver.log or login to webmin. I'm constantly
missing a commandline tool for this :-).
Comment 1 Peter Åstrand cendio 2004-01-20 10:37:22 CET
I usually use "ps auxww | grep xinit". Not a very nice solution either. 

There are lots of other cmd line tools missing as well, but I'll guess there's
no time to create them currently. 
Comment 4 William Sjöblom cendio 2022-02-01 14:43:56 CET
We have come up with some possible syntax for a future tool when
discussing bug 3707. The naming of these is up to discussion but one key
takeaway from our discussion is that we likely want to be relatively
verbose when naming both subcommands and flags in the beginning. The
subcommands/flags that seems to be the most commonly used later on can
then be given short-hands.

Whether to use singular or plural for the 'session' subcommand is up for
discussion, but we should try to follow conventions of other tools and
be consistent.


1 Listing sessions
══════════════════

 ┌────
 │ # Listing all sessions:
 │ tlctl session list
 │ # ID          USER    STATUS AGENT      LAST CONNECTION CREATED
 │ # 57fkgu730   wilsj   down   127.0.0.1  21 Feb 2022     19 Feb 2022
 │ # ...
 │
 │ # session ls accepts a --plain switch such that it only prints IDs.
 │ # Useful when scripting. Similar to the 'docker ps' --quiet flag.
 │ tlctl session list --plain
 │ # 57fkgu730
 │ # ...
 │
 │ # Showing only sessions running on a specific agent:
 │ tlctl session list --filter-agent="127.0.0.1"
 │
 │ # Listing sessions with given IDs. Maybe a good idea since we have a 
 │ # left-over positional argument. This makes `session ls' xargs friendly.
 │ tlctl session list 57fkgu730 57fkgu756 57fkgu733
 │ # ...
 └────


2 Terminating a session
═══════════════════════

  A couple of alternatives for terminating a session for a specific
  user:
 ┌────
 │ # Simple terminate
 │ tlctl session terminate --filter-user="wilsj"
 │ # somethingsomething? [Y/n] Y
 │
 │ # Simple terminate without confirmation prompt
 │ tlctl session terminate --filter-user="wilsj" -y
 │
 │ # Using xargs
 │ tlctl session list --filter-user="wilsj" --plain \
 │     | xargs tlctl session terminate -y
 └────
Comment 8 Frida Flodin cendio 2022-02-16 14:46:26 CET
Here are some user stories summarized from bug 3707 and some new ones that I could come up with now.

User stories
============

1. As an admin I have no idea if my ThinLinc system is used, I quickly
   want to see all sessions and their status from command line.

2. As an admin I have a user whose session is frozen for some reason. I
   want to quickly find out what agent the users session is on and the
   pid so that I can kill it.

3. As an admin, I want to list all running sessions so that I can take a
   decision on which agent I can upgrade first.

4. Someone calls to support and asks for a session that they could reach
   at time X, but can't anymore. Then I want to be able to check if that
   session is still running so that I know where to concentrate the
   troubleshooting effort.

5. We are about to decommission an agent, then I want to check that it
   doesn't have any running sessions, so the users are unaffected.

6. I want to see if my ThinLinc cluster is clean by listing all
   unreachable session so that I can take a decision on what to do with them.

7. From issue [1]:
   "[...] We were looking for a command-line tool that would allow us to
   query status of existing user sessions.  I.e. something that would
   print detailed session information as the "Sessions" page in the web
   GUI does... but from the command line, so we could script and
   automate things.  
   [...]  
   We were looking at gathering stats of our sessions (basically, four
   columns of "who, connection status, when started, when last
   connected").  This came up as we were deciding on the system use
   policies (values of -MaxDisconnectTime and the likes - so that we
   would not run over our licenses limit)."


[1] https://tracker.lkpg.cendio.se/tracker/issue25357
Comment 31 Linn cendio 2022-02-22 11:29:33 CET
User stories
============

1. A user has issues with their session. As an admin, I want to get the username, status and agent of the session to identify the session and get an overview of it. 

2. A user has issues with their session. As an admin, I want to get the process ID of the session so I can use it when troubleshooting.

3. A user has issues with their session. As an admin, I want to get the "ThinLinc ID" (agent, username, display number) so I can use this ID in other tlctl calls.


From these user stories, it suggests that the default headers of the table should be username, status, agent, process ID and display number.
Comment 34 Linn cendio 2022-02-24 09:23:28 CET
We have two different names for agents presented in webadm: 

1) Address as seen by the master ('agenthost')
2) Address as seen by the client ('termserv_hostname')

In webadm, 1) is the address used when assigning agents to subclusters, showing the load and the address used in vsmserver.log. 2) is used in the message after terminating a session, and in vsmagent.log if sometimes goes wrong when trying to terminate a session.

When assigning agents to subclusters the admin have to type in the addresses, so it is likely the type of address mostly associated with ThinLinc. For now we will use address type 1) as the presented agent address, but the most important thing is that the format of the agent address is consistent over tlctl.
Comment 37 Frida Flodin cendio 2022-02-25 16:40:57 CET
Regarding session verification
==============================

When getting the sessions from vsmserver you have the option to verify
the sessions against each agent. If the sessions are not verified
vsmserver replies with what's in the sessionstore right now. This session
data could be as old as 10 minutes [1]. We need to decide if and when
the sessions should be verified when listing sessions with tlctl.

In Webadm when you navigate to "Status" -> "Sessions" there is a list of
usernames and how many sessions they have. When printing this list Webadm
will not verify the sessions. When clicking on a username the detail of
the session is shown. When getting these details the session is
verified. This seems reasonable, in the overview list we have a rough
estimation of all sessions, but when viewing details for a specific user
the data is verified first.

When listing all sessions with tlctl right now, you see some details that
might be outdated. Here are some things that need to be discussed further:

1. Should we always verify the sessions listed with tlctl? Could this be
   a slow operation on large clusters? 
2. Should we have a rough list when listing all sessions that do not
   need to be verified? And not show any details in this list?
3. When using a filter for agent or user should we always verify?
4. Should we let the user of tlctl decide if the session should be
   verified or not, with a flag? And should the default be to not verify?



[1] This is set by configuring /vsmserver/session_update_delay, the
default is 10 minutes and the parameter is not documented so it should
be rare that this is changed in reality.
Comment 40 Linn cendio 2022-03-02 09:14:35 CET
We had a discussion about the work so far in tlctl, and here are the key points we talked about for listing sessions:

* List should show if a session is unreachable. It might be possible to show it as part of the STATUS header, otherwise we might have to introduce a new header.

* We should add some default sorting to ease the reading of the session table, e.g. sorting on the leftmost column (username) and display number.

* Regarding default headers: 'PID' will be removed as a default header, as that info can be derived from agent and display number, and we didn't find any immediate need for it in our user stories. Instead, make 'Created at Time' a default header, call it e.g. something short like AGE. It should show roughly how old the session is, e.g. '<5 min', '3 h', etc.

* Regarding error handling for the indata, we discussed if we should inform the user if they typed a username or agent that does not exist. For the agents, these names are only available in the conf, which in that case would make us dependent on having access to the conf, which is currently only on the master machine. The usernames would likely have to be checked among the local users on the master machine.

* As an extra feature, we are interested in adding filtering for subclusters in case we have time. However, this info is only available in the conf, making us further dependent on it.

* Right now, we are not interested in a 'session details' command (which would have shown all info about _one_ session). One issue is that we don't have any short, natural identifier for the sessions at the moment, which would make selecting a session a bit cumbersome. This is functionality we are interested in for the future though.
Comment 41 Linn cendio 2022-03-02 10:03:10 CET
Regarding verifying the sessions, it is not something we will include in this first version, but it might be interesting to add it eventually.


We also talked about adding a summary at the end of the list showing how many sessions were listed. This should be especially helpful for installations with very many users.
Comment 49 Linn cendio 2022-03-09 17:30:54 CET
We have discussed validating input data from our currently planned flags, i.e. usernames, agents and subclusters, and looked at how Tlwebadm retrieves its data.

Usernames
---------
Verified with pwd.getpwnam().

Since we are deriving the uid from the username, we want to limit the number of times we do the validating. Right now, we will settle for validating the username when we retrieve zero sessions, since it has to be a valid username when sessions were found.

We also have bug 2754 for changing from usernames to uids which would spare us the extra lookup step.


Agents and Subclusters
----------------------
The entries for both agents and subclusters are verified from the conf. 

One issue with this is that the conf only is in sync with vsmserver _if_ vsmserver has been restarted after any changes, meaning that there can be inconsistent information between the vsmserver and any program using the conf.

Another issue is that when draining an agent from sessions the agent is removed from the conf, meaning that an agent in this scenario cannot be validated. We have bug 1015 on this topic.
Comment 69 Linn cendio 2022-03-22 17:12:50 CET
This bug should now be ready for testing. These are the things I checked:

Acceptance criteria fulfilled:

> * It should be possible to list:
>   - All sessions
>   - All sessions for a user
>   - All sessions on an agent
>   - All sessions in a cluster
Yes, you can filter on all these parameters, and also on display number.


> * The listed information should contain:
>   - When the session was created
>   - Agent
>   - Username
>   - Display number
>   - Status
It does.


> * The list output should by default be sorted on username
It is, with sorting on display as the second sorting column.


Also tested that error messages look sane, and that the man and --help pages look okay.
Comment 70 Linn cendio 2022-03-23 09:38:11 CET
Currently, we have the prefix 'tlctl' for errors, but not for warnings. We should be consistent and add the prefix to the warnings as well.
Comment 73 Linn cendio 2022-03-23 13:22:30 CET
The warning messages now have the same format as the error messages.
Comment 74 Linn cendio 2022-03-24 11:22:28 CET
Clarifying the acceptance criteria to be clearer about what info the 'Status' header should contain.
Comment 76 William Sjöblom cendio 2022-03-24 14:34:02 CET
Since bug 7860 is currently open the acceptance criterion

* The listed information should contain:
  ...
  - Status
    - The user should be able to tell the connection status of a session or if a session is unreachable

cannot really be fully tested since an empty "Status" field could be due to other issues and not just bug 7860.

I'll continue testing the remaining acceptance criteria but will leave the one above until bug 7860 is resolved.
Comment 77 William Sjöblom cendio 2022-03-25 09:21:41 CET
I have partially tested `tlctl session list' and found some issues/bugs. Note that I am not done with the testing, but figured that I might as well dump my findings so far:

• Does not exit with a non-zero exit code if no sessions matching the
  provided filters were found. I find this unusual.

• In the rest of tlctl we use parenthetical plural. We should do this
  for the values in the "AGE" column as well. Ex. "1 hours" → "1
  hour(s)".

• Also in the "AGE" column, we abbreviate "minute" but not "hours". The
  "min" abbrev is also in singular while "hours" is in plural.

• Improvements are needed in "–help" summary text:
  • "info" → "information"
  • Potentially be more clear that we are listing information for all
    sessions running on a cluster. "List session info" could as well
    refer to listing the available information about a single session.

• Running `LC_ALL=C tlctl session list' with a non-ASCII username in the
  session store will result in the following output/stacktrace:
> ┌────
> │ USER    DISPLAY  AGENT    STATUS     AGE
> │ =============================================
> │ Traceback (most recent call last):
> │   File "/opt/thinlinc/sbin/tlctl", line 53, in <module>
> │     IIiIiiIiI ( )
> │   File "/opt/thinlinc/sbin/tlctl", line 49, in IIiIiiIiI
> │     I1i [ i1111IIi ] . execute ( ii1IiIiiII , I1I111i11I [ 1 : ] , Iio0 . get_subparser ( i1111IIi ) )
> │   File "/opt/thinlinc/modules/thinlinc/tlctl/session.py", line 278, in execute
> │     iIIIII1i111i ( options , args [ 1 : ] , IIII1iII11ii )
> │   File "/opt/thinlinc/modules/thinlinc/tlctl/session.py", line 48, in iIIIII1i111i
> │     tlctl . print_as_table ( oO0 , ii1IiIiiII )
> │   File "/opt/thinlinc/modules/thinlinc/tlctl/__init__.py", line 220, in print_as_table
> │     print ( IIiIIIII1iiI . format ( * OO0OOOoOOooO ) )
> │ UnicodeEncodeError: 'ascii' codec can't encode character '\xf6' in position 0: ordinal not in range(128)
> └────

• Running `LC_ALL=C tlctl session list --user=öl' (i.e. filtering by
  non-ASCII value on an ASCII system) will result in the following
  stacktrace:
> ┌────
> │ Traceback (most recent call last):
> │   File "/opt/thinlinc/sbin/tlctl", line 53, in <module>
> │     IIiIiiIiI ( )
> │   File "/opt/thinlinc/sbin/tlctl", line 49, in IIiIiiIiI
> │     I1i [ i1111IIi ] . execute ( ii1IiIiiII , I1I111i11I [ 1 : ] , Iio0 . get_subparser ( i1111IIi ) )
> │   File "/opt/thinlinc/modules/thinlinc/tlctl/session.py", line 278, in execute
> │     iIIIII1i111i ( options , args [ 1 : ] , IIII1iII11ii )
> │   File "/opt/thinlinc/modules/thinlinc/tlctl/session.py", line 35, in iIIIII1i111i
> │     ooo0O0oO00 = i1ii1 ( verify = False , filters = iiIII )
> │   File "/opt/thinlinc/modules/thinlinc/tlctl/session.py", line 18, in i1ii1
> │     ooo0O0oO00 = tlctl . master_rpc ( "get_sessions" , verify , filters )
> │   File "/opt/thinlinc/modules/thinlinc/tlctl/__init__.py", line 39, in master_rpc
> │     IIII11 = getattr ( i11IiIIiii1II , method ) ( * args )
> │   File "/usr/lib64/python3.4/xmlrpc/client.py", line 1105, in __call__
> │     return self.__send(self.__name, args)
> │   File "/usr/lib64/python3.4/xmlrpc/client.py", line 1439, in __request
> │     allow_none=self.__allow_none).encode(self.__encoding)
> │ UnicodeEncodeError: 'utf-8' codec can't encode character '\udcc3' in position 205: surrogates not allowed
> └────
Comment 78 William Sjöblom cendio 2022-03-25 09:27:45 CET
Note that `tlctl session terminate` in bug 7833 likely also suffer from the crashes in the last two points of comment 77.
Comment 79 William Sjöblom cendio 2022-03-25 13:43:09 CET
I have tested the following things using server build #2535. This was
done on a 4 node ThinLinc cluster: one node running both an agent and
the master while the others were exclusive agent nodes. Two of these
agent nodes were configured in a common subcluster while the remaining
two agent nodes had their own subclusters. Testing was performed
with all nodes running SLES12 to make sure that we have no python 3.4
incompatibilities.

• Input data character encoding ≠ tlctl character encoding (see comment 77)
• Output data character encoding ≠ tlctl character encoding (see comment 77)
• Using tlctl when master is not running → error message
• Using tlctl on a node that is not running `vsmserver' → error message
• Truncation

  Listing sessions with a 32 character username and an artifically long
  hostname defined through `/etc/hosts'. Truncation looks good and lines
  are capped at 80 columns.
  
• Single user with multiple sessions → shows up as expected

Acceptance criteria:
• ☒ It should be possible to list:
  • ☑ All sessions

    Yes!

  • ☐ All sessions for a user

    Yes, if we exclude issues when running:
>   ┌────
>   │ LC_ALL=C tlctl session list --user=ö
>   └────

  • ☑ All sessions on an agent

    Yes!

  • ☐ All sessions in a subcluster

    Yes, if we exclude the issues when running:
>   ┌────
>   │ LC_ALL=C tlctl session list --subcluster=ö
>   └────


• ☒ The listed information should contain:
  • ☐ When the session was created

    Yes, if we exclude the inconsistensies mentioned in comment 77.

  • ☑ Agent

    Yes!

  • ☐ Username

    Yes, if we exclude the locale issues when running (with non-ASCII
    usernames in the sessionstore):
>   ┌────
>   │ LC_ALL=C tlctl session list
>   └────


  • ☑ Display number

    Yes!

  • ☐ Status

    I was not able to test this due to bug 7860.

• ☑ The list output should by default be sorted on username

  Yes!

I am currently in the midst of evaluating `tlctl session list' against
https://clig.dev. When that's done I have a few corners of the code
left to scrutinize.
Comment 80 William Sjöblom cendio 2022-03-28 09:56:11 CEST
We should probably mention on the man page that due to us not verifying the listed session, the data presented may not be up-to-date.
Comment 81 Linn cendio 2022-03-29 10:11:09 CEST
(In reply to William Sjöblom from comment #77)
> ...
> • Running `LC_ALL=C tlctl session list --user=öl' (i.e. filtering by
>   non-ASCII value on an ASCII system) will result in the following
>   stacktrace:
> > ...
I can reproduce this traceback with Python 3.4 (SLES 12) but not with Python 3.10 (Fedora 35).

Looking at the xmlrpc code, this change in 3.5 seems to be the reason for this behaviour in client.py:
https://github.com/python/cpython/commit/aebb6d3682e08c93d8468a9291180c5cbdc2df1b


However, when running the command with Python 3.6 (RHEL 8), I get this error in the terminal:
> tlctl: error: an error occured when contacting the master: Remote end closed connection without response
> See /var/log/vsmserver.log for more information.
Followed by a traceback in the log file:
> 2022-03-28 14:42:19 ERROR vsmserver: Unhandled XMLRPC exception: <class 'thinlinc.vsm.xmlrpc.XMLDeMarshallingError'> ['reference to invalid character number: line 12, column 15', b"<?xml version='1.0'?>\n<methodCall>\n<methodName>get_sessions</methodName>\n<params>\n<param>\n<value><boolean>0</boolean></value>\n</param>\n<param>\n<value><struct>\n<member>\n<name>username</name>\n<value><string>&#56515;&#56502;l</string></value>\n</member>\n</struct></value>\n</param>\n</params>\n</methodCall>\n"] Traceback (most recent call last):
>   File "/opt/thinlinc/modules/thinlinc/vsm/xmlrpc.py", line 565, in handle_request
>     self . params , self . methodname = loads ( self . payload )
>   File "/usr/lib64/python3.6/xmlrpc/client.py", line 1019, in loads
>     p.feed(data)
>   File "/usr/lib64/python3.6/xmlrpc/client.py", line 439, in feed
>     self._parser.Parse(data, 0)
> xml.parsers.expat.ExpatError: reference to invalid character number: line 12, column 15
> 
> During handling of the above exception, another exception occurred:
> 
> Traceback (most recent call last):
>   File "/opt/thinlinc/modules/thinlinc/vsm/asyncbase.py", line 104, in ooOOOoOO0
>     obj . handle_read_event ( )
>   File "/usr/lib64/python3.6/asyncore.py", line 423, in handle_read_event
>     self.handle_read()
>   File "/usr/lib64/python3.6/asynchat.py", line 151, in handle_read
>     self.found_terminator()
>   File "/opt/thinlinc/modules/thinlinc/vsm/xmlrpc.py", line 555, in found_terminator
>     self . handle_request ( )
>   File "/opt/thinlinc/modules/thinlinc/vsm/xmlrpc.py", line 567, in handle_request
>     raise XMLDeMarshallingError ( [ str ( o0o0Oo00000o ) , self . payload ] )
> thinlinc.vsm.xmlrpc.XMLDeMarshallingError: ['reference to invalid character number: line 12, column 15', b"<?xml version='1.0'?>\n<methodCall>\n<methodName>get_sessions</methodName>\n<params>\n<param>\n<value><boolean>0</boolean></value>\n</param>\n<param>\n<value><struct>\n<member>\n<name>username</name>\n<value><string>&#56515;&#56502;l</string></value>\n</member>\n</struct></value>\n</param>\n</params>\n</methodCall>\n"]

When checking the fix, it should be tested on all these Python versions.
Comment 88 Linn cendio 2022-03-31 10:45:03 CEST
(In reply to William Sjöblom from comment #77)
> ...
> • Running `LC_ALL=C tlctl session list --user=öl' (i.e. filtering by
>   non-ASCII value on an ASCII system) will result in the following
>   stacktrace:
> > ...
This stacktrace has now been fixed by adding a check in our custom optparse to abort if the input string contains surrogate pairs. The surrogate pairs are unicode code points in a certain range that Python uses to indicate that the string has unknown encoding. 

Tester should check that we no longer crash and that we output a sane error message. Since we no longer enter the xmlrpc code that differed for different Python versions, I think it's enough to test this on one of the dists where the error could be reproduced.
Comment 89 Linn cendio 2022-03-31 13:43:14 CEST
(In reply to William Sjöblom from comment #77)
> ...
> • Running `LC_ALL=C tlctl session list' with a non-ASCII username in the
>   session store will result in the following output/stacktrace:
> > ...

This can be reproduced on Python 3.6 (RHEL 8) but not with Python 3.9 (Fedora 35).

This reason for the different behaviours seems to be this change in Python 3.7:
https://docs.python.org/3/library/os.html#utf8-mode
Comment 91 Linn cendio 2022-04-05 11:28:38 CEST
(In reply to William Sjöblom from comment #77)
> ...
> • Running `LC_ALL=C tlctl session list' with a non-ASCII username in the
>   session store will result in the following output/stacktrace:
> > ...

Tested on RHEL 8 after the fix, and we now print '?' for unprintable characters instead of crashing.
Comment 93 Linn cendio 2022-04-06 15:45:47 CEST
(In reply to William Sjöblom from comment #77)
> I have partially tested `tlctl session list' and found some issues/bugs.
> ...
> 
> • Does not exit with a non-zero exit code if no sessions matching the
>   provided filters were found. I find this unusual.

We discussed this and the exit code will be left as-is for the moment. E.g. docker uses this exit code too when listing containers.
Comment 98 Linn cendio 2022-04-07 08:53:41 CEST
This bug is now ready to be retested after a passing Jenkins build. The things that are left to test are:

1. Column AGE should use plural in parenthesis were applicable. Minutes have still been left as the abbreviated 'min'.

2. The helptext and man page should look ok.

3. Locale issues should have been solved. Note that this only can be seen on Python <= 3.6.

 a) Non-ascii username in session database and running 'LC_ALL=C sudo tlctl session list'.
    Tester should check that we now print '?' for unprintable chars instead of crashing.
    
 b) Filtering by non-ascii value on ascii system by running 'LC_ALL=C sudo tlctl session list --user=öl'
    Tester should check that we no longer crash and that we output a sane error message.
	
4. The acceptance criteria tests that couldn't be tested in comment 79. Most of them are related to the locale issues, except for one related empty status values due to bug 7860. All of these should be solved by now.
Comment 100 Linn cendio 2022-04-07 10:49:59 CEST
The Jenkins build was successful, marking as resolved.
Comment 101 William Sjöblom cendio 2022-04-07 17:16:05 CEST
Overall things look good. I have checked the following using server
build #2561 running on a SLES12 cluster:

1. Plural forms

   At least wiktionary seem to think that the plural form of "min" is
   "mins" (<https://en.wiktionary.org/wiki/min#Noun>).

   The other changes look good.

2. `--help' and `man tlctl-session'

   I have submitted some minor feedback regarding the help text over
   email. Other both the `--help' page and man page look good.

3. Non-ascii username in session db / filtering by non-ascii username

   Tested these along with various other locale combinations without
   issues. Handling of different encodings seems robust now!

4. The user should be able to tell the connection status of a session or
   if a session is unreachable

   Does a good job of indicating the session status. "unreachable"
   takes precedence over the connection status. Additionally, this was
   partially tested for bug 7860.


I have also had a thorough look through the changes and emailed some
minor feedback. Other than that things look perfectly fine!

When the minor issues are fixed, we should be good to go. Reopening in
the meantime.
Comment 103 Linn cendio 2022-04-11 12:39:42 CEST
(In reply to William Sjöblom from comment #101)
> Overall things look good. I have checked the following using server
> build #2561 running on a SLES12 cluster:
> 
> 1. Plural forms
Both 'min' and 'min(s)' seem to be common abbreviations for minutes. Even if 'min(s)' would fit better in with the other time units, I still think 'min' looks more natural. Will leave the abbreviation as 'min' for now.


> 2. `--help' and `man tlctl-session'
The improved help text suggestion has been committed.


Also looked through the code suggestion in the email, but won't break out the stdout/stderr handling to a separate function at this time. 

Marking as resolved
Comment 105 William Sjöblom cendio 2022-04-12 11:06:29 CEST
Looks good now. Closing!
Comment 106 Linn cendio 2023-03-21 14:06:25 CET
In the load module we sort agent names based on locale, but we don't do that for the session module.

We should update the session module to sort it sessions based on locale. Load does it using this built-in function:
> locale.strxfrm
Comment 108 Linn cendio 2023-03-24 14:51:37 CET
Tested on RHEL 8 with German locale (de_DE) which has alphabet order: 
  a, ä, ..., o, ö, ..., z

* Order before fix:
USER  DISPLAY  AGENT      STATUS     AGE   
===========================================
aser  12       127.0.0.1  connected  24 min
zser  10       127.0.0.1  connected  25 min
äser  11       127.0.0.1  connected  24 min


* Order after fix:
USER  DISPLAY  AGENT      STATUS     AGE   
===========================================
aser  12       127.0.0.1  connected  23 min
äser  11       127.0.0.1  connected  24 min
zser  10       127.0.0.1  connected  24 min

The order of the usernames now matches the alphabetical order, marking as resolved.

* Tips for tester
Note that changes to locale is not updated immediately, I restarted my machine to trigger it.
Comment 109 Tobias cendio 2023-03-30 09:43:35 CEST
Tested server builds #3157 (pre-fix) and #3168 (post-fix) on Ubuntu22.04.

#3157 (pre-fix)
---------------
locale: de_DE
USER    DISPLAY  AGENT      STATUS     AGE  
============================================
user_a  12       127.0.0.1  connected  1 min
user_o  14       127.0.0.1  connected  1 min
user_u  16       127.0.0.1  connected  1 min
user_z  11       127.0.0.1  connected  2 min
user_ä  15       127.0.0.1  connected  1 min
user_ö  10       127.0.0.1  connected  5 min
user_ü  13       127.0.0.1  connected  1 min

locale: sv_SE
USER    DISPLAY  AGENT      STATUS     AGE  
============================================
user_a  15       127.0.0.1  connected  1 min
user_o  16       127.0.0.1  connected  1 min
user_u  14       127.0.0.1  connected  1 min
user_z  12       127.0.0.1  connected  2 min
user_ä  10       127.0.0.1  connected  2 min
user_ö  13       127.0.0.1  connected  1 min
user_ü  11       127.0.0.1  connected  2 min

#3168 (post-fix)
----------------
locale: de_DE
USER    DISPLAY  AGENT      STATUS     AGE   
=============================================
user_a  15       127.0.0.1  connected  1 min 
user_ä  10       127.0.0.1  connected  1 min 
user_o  11       127.0.0.1  connected  1 min 
user_ö  13       127.0.0.1  connected  1 min 
user_u  14       127.0.0.1  connected  1 min 
user_ü  16       127.0.0.1  connected  <1 min
user_z  12       127.0.0.1  connected  1 min

locale: sv_SE
USER    DISPLAY  AGENT      STATUS     AGE  
============================================
user_a  15       127.0.0.1  connected  1 min
user_o  16       127.0.0.1  connected  1 min
user_u  14       127.0.0.1  connected  1 min
user_ü  11       127.0.0.1  connected  2 min
user_z  12       127.0.0.1  connected  1 min
user_ä  10       127.0.0.1  connected  2 min
user_ö  13       127.0.0.1  connected  1 min

Contrary to build #3157 which lacked locale-based session sorting, build #3168 lists sessions according to the expected locale alphabetical order. Closing.

Note You need to log in before you can comment on or make changes to this bug.