Autocompletion of ioc and version #60

marcelldls · 2023-11-27T13:09:04Z

Possibly resolves #46 - have only applied to deploy command

marcelldls · 2023-11-28T10:28:18Z

marcelldls · 2023-11-28T10:42:23Z

The performance is not amazing - some caching would help speed up subsequent calls

gilesknap

This looks really promising. One other thing we need completion on is files in the file system.
e.g. when you type:
ec ioc deploy-local iocs/ tab tab

I had hoped that typer would have built in support for this but I did not find it when I looked, so you may have to resort to implementing your own using PathLib.

coretl · 2023-11-30T13:14:21Z

src/epics_containers_cli/git.py

    tag
    """
    check_beamline_repo(beamline_repo)
-    typer.echo(f"Available instance versions for {ioc_name}:")
-
    run_command(f"git clone {beamline_repo} {folder}", interactive=False)


shallow clone would probably help here

Also, as this is a cli, we could lazily clone this the first time we need it, grab the info out of it and store it as a global, then use it multiple times

Or we could make it a pure function with no side effects (i.e. clone, get instance and version info, delete, then return info) and cache it

I believe we need the full history to get all the available tags for each IOC so I don't believe shallow clone is an option. Im not sure the global works as each auto complete is a new run of the code, or is there a nice way to have persistent variables i am not aware of? Giles and I did discuss creating a graph of IOCs and versions once and just sharing it as you have mentioned. I have an attempt at pure functions like you have descibed and I think it works quite well but I will try a graph approach as well

If you only need to list the tags, what about git ls-remote --tags <repo>?

Its not just listing the tags - there is a relationship between the tags and last changes to the IOC

* Attempt caching for autocomplete * Create available IOC & version graph

gilesknap · 2023-12-04T15:57:44Z

@marcelldls this looks really good. I have tried it and it works well.

Things still to do (you probably already know this but let's make sure we are on the same page):

add ioc name completion to these ioc subcommands using kubectl to query which are available. This one is a bit challenging because we need to use the "local_commands" instead of "k8s_commands" approach if the EC_K8S_DOMAIN==local (you could ignore this for now as it's only for non dls-users and the tutorials):
- attach
- delete
- exec
- log-history
- logs
- restart
- start
- stop
- template
add filename completion to
- deploy-local
- validate
- template
add version completion to deploy when you have specified a local folder instead of a name (I think that is correct - maybe we could use the version from the yaml file if none is supplied but is that assuming too much?)

marcelldls · 2023-12-05T10:17:24Z

This looks really promising. One other thing we need completion on is files in the file system. e.g. when you type: ec ioc deploy-local iocs/ tab tab

I had hoped that typer would have built in support for this but I did not find it when I looked, so you may have to resort to implementing your own using PathLib.

fastapi/typer#663

GDYendell · 2023-12-05T10:51:42Z

fastapi/typer#663

From that issue:

The workaround I found is to set the type to str instead of Path but I'm not satisfied with it.

That can't be right surely? If you use str it autocompletes files from the current directory?

marcelldls · 2023-12-05T13:27:08Z

tiangolo/typer#663

From that issue:

The workaround I found is to set the type to str instead of Path but I'm not satisfied with it.

That can't be right surely? If you use str it autocompletes files from the current directory?

I believe if it expects a string but you dont give it a list of strings to use for auto complete it reverts to some behavior which is a path autocompletion

I think this is a click issue which is broken for Path type arguments - which should be giving nice path autocompletion. The problem with the work around to use str arguments is that you dont get the built in path validation. I have found my own work around which is to give an autocompletion function which returns [] - this seems to force path autocompletion and still retain the path validation

gilesknap · 2023-12-06T13:49:28Z

have found my own work around which is to give an autocompletion function which returns [] - this seems to force path autocompletion and still retain the path validation

Let's make sure we get a minimum reproduction of this issue reported to typer, please.

gilesknap · 2023-12-06T14:16:50Z

Just trying this out and it looks freakin awesome!

Some pedantry:

ec ioc start shows already started iocs (but starting an already started IOC is benign)
ec ioc logs lists all ioc deployments but it hangs when logging a non-running deployment

I just checked and kubectl -n i20-1-iocs logs deploy/bl20j-di-phdgn-05 does time out if there is no pod running. Probably best to only show running IOCS for 'logs' . This is probably expected behaviour - deployments with single pods can log that pod - deployments with no pods to log probably wait for one to come up.

gilesknap

I'm happy with this now.

I feel like we could go ahead and merge, then add some unit tests when you return from vacation.

gilesknap · 2023-12-08T08:05:51Z

@marcelldls much better error handling.

One niggle:

(ec-venv) (python3.11) (ioc-autocomplete) [hgv27681@pc0116 epics-containers-cli]$ ec ioc exec Please set EC_K8S_NAMESPACE or pass --namespace
Please set EC_K8S_NAMESPACE or pass --namespace

docs/                    get_helm.sh              .github/                 LICENSE                  pyproject.toml           src/                     .vscode/                 
environment.sh           .git/                    .gitignore               .pre-commit-config.yaml  README.rst               tests/                   
(ec-venv) (python3.11) (ioc-autocomplete) [hgv27681@pc0116 epics-containers-cli]$ ec ioc exec

We still get the file completion after printing the error message, which could be confusing.

gilesknap · 2023-12-08T08:25:24Z

Something is horribly wrong:

(ec-venv) (python3.11) (dev) [hgv27681@pc0116 bl20j]$ ec ioc deploy-local iocs/bl20j-
bl20j-di-phdgn-02/ bl20j-di-phdgn-03/ bl20j-di-phdgn-05/ bl20j-di-phdgn-07/ bl20j-di-phdgn-08/ bl20j-mo-brick-06/ 
(ec-venv) (python3.11) (dev) [hgv27681@pc0116 bl20j]$ ec ioc deploy bl20j-
bl20j-di-ioc-01    bl20j-ea-phdgn-02  bl20j-ea-phdgn-03  bl20j-ea-phdgn-05  bl20j-ea-phdgn-07  bl20j-ea-phdgn-08

What I have in my iocs folder and what is available to deploy disagree - this is probably my issue rather than yours.

marcelldls · 2023-12-08T10:23:51Z

@marcelldls much better error handling.

One niggle:

(ec-venv) (python3.11) (ioc-autocomplete) [hgv27681@pc0116 epics-containers-cli]$ ec ioc exec Please set EC_K8S_NAMESPACE or pass --namespace
Please set EC_K8S_NAMESPACE or pass --namespace

docs/                    get_helm.sh              .github/                 LICENSE                  pyproject.toml           src/                     .vscode/                 
environment.sh           .git/                    .gitignore               .pre-commit-config.yaml  README.rst               tests/                   
(ec-venv) (python3.11) (ioc-autocomplete) [hgv27681@pc0116 epics-containers-cli]$ ec ioc exec

We still get the file completion after printing the error message, which could be confusing.

Fair enough. A difficulty is if you dont want that you have to give something to autocomplete...
I do think I have a better approach now though to rather get

(ec-dev) (base) vboxuser@Ubuntu-dev:~/WIP/epics-containers-cli$ ec ioc attach Please set EC_K8S_NAMESPACE or pass --namespace
  Please set EC_K8S_NAMESPACE or pass --namespace
  Please set EC_K8S_NAMESPACE or pass --namespace
  Please set EC_K8S_NAMESPACE or pass --namespace

But I dont really understand fully why it works

gilesknap · 2023-12-08T10:46:42Z

(ec-dev) (base) vboxuser@Ubuntu-dev:~/WIP/epics-containers-cli$ ec ioc attach Please set EC_K8S_NAMESPACE or pass --namespace
  Please set EC_K8S_NAMESPACE or pass --namespace
  Please set EC_K8S_NAMESPACE or pass --namespace
  Please set EC_K8S_NAMESPACE or pass --namespace

Yep I think that is better.

gilesknap · 2023-12-08T10:48:47Z

So I got my bl20j repo to look OK:

(ec-venv) (python3.11) (dev) [hgv27681@pc0116 bl20j]$ ec ioc deploy-local iocs/bl20j-
bl20j-di-phdgn-02/ bl20j-di-phdgn-03/ bl20j-di-phdgn-05/ bl20j-di-phdgn-07/ bl20j-di-phdgn-08/ bl20j-mo-brick-06/ 
(ec-venv) (python3.11) (dev) [hgv27681@pc0116 bl20j]$ ec ioc deploy bl20j-
bl20j-di-phdgn-02  bl20j-di-phdgn-03  bl20j-di-phdgn-05  bl20j-di-phdgn-07  bl20j-di-phdgn-08  bl20j-mo-brick-06

The thing that I did was push my changes to main (from dev). It's not clear to me why this was needed. I have been putting my tags on the dev branch for that repo (and probably should not be doing that!). Perhaps I will allow working in the main branch for beamline repos anyway - anything else seems to make extra work.

* More general ioc versions function * Add autocomplete skeleton * More skeleton code * Implement autocomplete * Linting * Ioc autocomplete cached (#2) * Attempt caching for autocomplete * Create available IOC & version graph * Add catch for version autocomplete * Remove caching from ioc instances call * Handle --repo option * Implement unique cache per domain repo * Ignore files present in iocs/ * Add workaround for path autocompletion * Add kubectl autocompletions * Change ec logs autocomplete to running iocs * Add some typing + error handling for autocomplete * Ignore typer deprecated warning * Make mypy happier * Linting * Make autocomplete keyerror behavior consistant * Autocomplete with error rather than files * fix instances test * Fix f-string for failed test * Include directory context manager addition --------- Co-authored-by: Giles Knap <[email protected]>

marcelldls added 5 commits November 27, 2023 10:55

More general ioc versions function

fb8722f

Add autocomplete skeleton

29b2b93

More skeleton code

e990107

Implement autocomplete

40885f4

Linting

1b385d5

marcelldls requested a review from gilesknap November 28, 2023 10:45

gilesknap reviewed Nov 29, 2023

View reviewed changes

coretl reviewed Nov 30, 2023

View reviewed changes

marcelldls and others added 5 commits December 1, 2023 09:16

Ioc autocomplete cached (#2)

a92f119

* Attempt caching for autocomplete * Create available IOC & version graph

Add catch for version autocomplete

556acc5

Remove caching from ioc instances call

525ff51

Handle --repo option

02fbb18

Implement unique cache per domain repo

62564ea

Ignore files present in iocs/

e4bc807

marcelldls added 2 commits December 5, 2023 13:51

Add workaround for path autocompletion

0ccda28

Add kubectl autocompletions

aa27029

Change ec logs autocomplete to running iocs

06b64ab

gilesknap approved these changes Dec 6, 2023

View reviewed changes

marcelldls added 4 commits December 7, 2023 12:59

Add some typing + error handling for autocomplete

f78cb59

Ignore typer deprecated warning

788bd06

Make mypy happier

2735302

Linting

f088fb2

marcelldls marked this pull request as ready for review December 7, 2023 16:41

marcelldls added 2 commits December 8, 2023 10:02

Make autocomplete keyerror behavior consistant

bc0470e

Autocomplete with error rather than files

6b15186

gilesknap and others added 4 commits December 8, 2023 13:35

fix instances test

51034b0

Fix f-string for failed test

042248d

Include directory context manager addition

b3f94f2

Merge branch 'main' into ioc-autocomplete

a0c5978

marcelldls merged commit cbccded into epics-containers:main Dec 8, 2023
7 checks passed

marcelldls deleted the ioc-autocomplete branch March 1, 2024 08:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Autocompletion of ioc and version #60

Autocompletion of ioc and version #60

marcelldls commented Nov 27, 2023

marcelldls commented Nov 28, 2023

marcelldls commented Nov 28, 2023

gilesknap left a comment

coretl Nov 30, 2023

coretl Nov 30, 2023

coretl Nov 30, 2023

marcelldls Nov 30, 2023

GDYendell Nov 30, 2023 •

edited

Loading

marcelldls Nov 30, 2023

gilesknap commented Dec 4, 2023

marcelldls commented Dec 5, 2023

GDYendell commented Dec 5, 2023

marcelldls commented Dec 5, 2023

gilesknap commented Dec 6, 2023

gilesknap commented Dec 6, 2023

gilesknap left a comment

gilesknap commented Dec 8, 2023

gilesknap commented Dec 8, 2023 •

edited

Loading

marcelldls commented Dec 8, 2023

gilesknap commented Dec 8, 2023

gilesknap commented Dec 8, 2023

Autocompletion of ioc and version #60

Autocompletion of ioc and version #60

Conversation

marcelldls commented Nov 27, 2023

marcelldls commented Nov 28, 2023

marcelldls commented Nov 28, 2023

gilesknap left a comment

Choose a reason for hiding this comment

coretl Nov 30, 2023

Choose a reason for hiding this comment

coretl Nov 30, 2023

Choose a reason for hiding this comment

coretl Nov 30, 2023

Choose a reason for hiding this comment

marcelldls Nov 30, 2023

Choose a reason for hiding this comment

GDYendell Nov 30, 2023 • edited Loading

Choose a reason for hiding this comment

marcelldls Nov 30, 2023

Choose a reason for hiding this comment

gilesknap commented Dec 4, 2023

marcelldls commented Dec 5, 2023

GDYendell commented Dec 5, 2023

marcelldls commented Dec 5, 2023

gilesknap commented Dec 6, 2023

gilesknap commented Dec 6, 2023

gilesknap left a comment

Choose a reason for hiding this comment

gilesknap commented Dec 8, 2023

gilesknap commented Dec 8, 2023 • edited Loading

marcelldls commented Dec 8, 2023

gilesknap commented Dec 8, 2023

gilesknap commented Dec 8, 2023

GDYendell Nov 30, 2023 •

edited

Loading

gilesknap commented Dec 8, 2023 •

edited

Loading