Re: master 55eabe96c9: ; Improve manual for Tramp kubernetes method

[Michael Albinus <michael.albinus@gmx.de>]

Filipp Gunbin <fgunbin@fastmail.fm> writes:

> Hi Michael,

Hi Filipp,

> Generally it works, thank you very much!

Thanks for checking!

> Test case:
>
> 1) kubectl config set-context --current --namespace=main
> 2) dired /kubernetes:myapp-67cd597654-wxdjn:/
> 3) tramp-cleanup-this-connection
> 4) kubectl config set-context --current --namespace=
> 5) again dired /kubernetes:myapp-67cd597654-wxdjn:/ - I get an error and
>   cache is cleared
>
> However, now auto-completing /kubernetes: shows this, why 2?
>
> In this buffer, type RET to select the completion near point.
>
> 2 possible completions:
> kubernetes:

Hmm. It says 2 possible completions, but doesn't show anything. Not sure
what this means.

Anayway, I've added a new user option to Tramp,
tramp-completion-use-cache. If you set it to nil, Tramp doesn't use
cached connections for host name completion. This shall fix your use case.

> Another issue: if at step 5 I instead use existing dired window and
> type, say, "i" on a subdirectory, then I get the same-looking error, but
> the Tramp cache is not cleared (perhaps this is not
> Kubernetes-specific).

Cannot say anything, let's see whether it is still present after the
following changes.

>> +(defun tramp-kubernetes--current-context-data (vec)
>> +  "Return current context data as JSON string."
>> +  (with-temp-buffer
>> +    (when (zerop
>> +	   (tramp-call-process
>> +	    vec tramp-kubernetes-program nil t nil
>> +	    "config" "view" "--minify" "-o" "json"))
>> +      (buffer-string))))
>> +
>
> Let's remove --minify.
>
> Also, maybe save the checksum (simple md5 would do it), not the full
> listing?  The listing may contain sensitive data (for example, mine
> shows my OAuth id token).

Right, there's too much data. And Tramp shouldn't expose sensible
data. Finally, I've changed this to

--8<---------------cut here---------------start------------->8---
# kubectl config view -o jsonpath='{.contexts[?(@.name == "<current-context-name>")]}'
--8<---------------cut here---------------end--------------->8---

with <current-context-name> being the name of the current context. This
shows just the fields in the current context, and nothing else - that's
exactly what we need for the check.

>> +    (when (and (stringp old-config-check) (stringp config-check)
>> +	       (not (string-equal old-config-check config-check)))
>> +      (tramp-message
>> +       vec 3
>> +       "Connection reset, because remote configuration changed from `%s' to `%s'"
>> +       old-config-check config-check)
>
> Currently, we'll get huge config listings here (at the level 3)?  If we
> change to checksums, that'd be not very useful.  So perhaps don't output
> those strings?

With the change above, the check string is much shorter. Yes, we could
use checksums, but this will prevent us to understand why the connection
cache has been cleared.

And this shouldn't happen too often, so I guess a message of level 3 is
still useful and not too annoying.

>> +  * `tramp-config-check'
>> +    A function to be called with one argument, VEC.  It should
>> +    return a string which is used to check, whether the
>
> Comma not needed?

Don't know. In the Tramp sources, both "check, whether" and "check
whether" is used, so it is inconsistent at all :-) Let's keep it, until
somebody teaches me to change it this way or the other in all places.

>>    * `tramp-copy-program'
>>      This specifies the name of the program to use for remotely copying
>>      the file; this might be the absolute filename of scp or the name of
>
> Also, my added paragraph in manual can be removed now.

Done.

I've committed my last version of the patch, because I believe it is
more simple to discuss about. Further comments welcome!

> Thanks.
> Filipp

Best regards, Michael.