List Workspaces CLI command #841

jprochazk · 2022-09-27T15:27:04Z

https://keboola.atlassian.net/browse/KAC-232

Changes:

Implement remote workspace list command

jprochazk · 2022-09-27T15:28:21Z

Tady vůbec netuším, jak to otestovat. Ty sandboxy jsou uložený mimo projekt (nejsou to configy).

michaljurecko · 2022-09-27T15:30:34Z

Otestovat manualne a k testom sa vratit, ked bude implementovany aj delete.
Idem to vyskusat.

michaljurecko · 2022-09-27T15:35:54Z

@jprochazk asi bude potrebne ignorovat nevalidne configs (?)

Tj, asi to idealne zalogovat ako DEBUG hlasku.

Je to tento projekt, ak by si to chcel skusit u seba:
https://connection.north-europe.azure.keboola.com/admin/projects/238

(V chybovej hlaske by bolo super mat ID konfiguracie)

jprochazk · 2022-09-28T10:20:57Z

Změny:

Odstranil jsem ten test a vytvořil pro to task https://keboola.atlassian.net/browse/KAC-250
invalid workspace error je teď debug hláška
Output má každý workspace na jednom řádku
Load configů a instances teď běží parallelně

pkg/lib/operation/project/remote/workspace/list.go

jprochazk · 2022-10-07T12:54:18Z

Netuším, proč failuje lint. Lokálně mi běží.

jprochazk · 2022-10-07T12:58:48Z

Aha, vidím, že jsme vyměnili goimports za gci

jprochazk · 2022-10-07T13:05:17Z

Ten ale nedokážu nastavit v IDE, mám tam jen tyhle možnosti:

@JakubMatejka

JakubMatejka · 2022-10-07T13:14:05Z

No je tam špatný pořadí, make fix s tím udělá tohle:

JakubMatejka · 2022-10-07T13:15:07Z

Co se týče nastavení IDE, tak v IntelliJ mám goimports ale je to manuálně donastavený takto:

JakubMatejka · 2022-10-07T13:22:40Z

Ale dívám se že ta záměna za gci proběhla v květnu kvůli tomu že to tehdy nefungovalo s Go 1.18. Teď ale nemůžu nikde dogooglit že by to měl být problém. Tak možná můžeme zpátky na goimports? @michaljurecko (Nevím moc jakej je v tom rozdíl kromě toho že goimports je v oficiálním repu)

JakubMatejka · 2022-10-07T13:24:15Z

BTW. máš nějakej divnej email v git logu, máš to dobře nastavený?

jprochazk · 2022-10-07T13:25:35Z

Ten email je vygenerovaný od githubu, je to anti-spam measure.

michaljurecko · 2022-10-07T13:45:51Z

Je to nastavene tu:

goimports nefunguju vzdy dobre, vid napr.:
golang/go#20818

Najlepsie by bolo nastavit si visual studio aby pouzivalo golangci-lint
https://golangci-lint.run/usage/integrations/
(mozno bude potrebne pridat aj cestu ku config suboru, alebo ho presunut do project root)

Staci takto?

jprochazk · 2022-10-07T14:44:47Z

Proč mi lokálně nefailuje make lint?

JakubMatejka · 2022-10-07T14:51:45Z

To se mi ale vlastně taky někdy dělo. 🤔 Myslel jsem že mám lokálně nějakou starší verzi golinteru, ale vlastně nevím.

jprochazk · 2022-10-07T14:52:56Z

golangci-lint mám v1.50.0

jprochazk · 2022-10-07T14:53:33Z

Ty options pro import sorting vscode nemá.

michaljurecko · 2022-10-07T14:53:59Z

@jprochazk @JakubMatejka neviem, mozete skusit prosim spustit gci priamo?

go install github.com/daixiang0/gci@latest

gci diff -s "standard,default,prefix(github.com/keboola/keboola-as-code)" ./pkg/lib/operation/project/remote/workspace/create.go

Ak to pojde ocakavane, tak skuste vypnut v nastaveniach ostatne linters a spustit to.
Ak pojde aj to, tak je potrebne najst, ktory iny linter robi problemy, napr. mozno to vyriesi vypnutie gofumpt.

JakubMatejka · 2022-10-07T14:54:03Z

Myslím to teda tak že lokálně nic, ale až v Docker kontejneru to začalo hlásit chybu.

jprochazk · 2022-10-07T14:55:00Z

michaljurecko · 2022-10-07T14:55:40Z

Verzia?

jprochazk · 2022-10-07T14:55:46Z

Btw, language server mi hlásí error

[Error - 4:52:26 PM] 2022/10/07 16:52:26 tidy: diagnosing file:///home/jan/work/keboola-as-code/go.mod: err: exit status 1: stderr: go: go.mod file indicates go 1.19, but maximum version supported by tidy is 1.18

jprochazk · 2022-10-07T14:56:00Z

JakubMatejka · 2022-10-10T08:49:19Z

Jo to se může stát. Není to standardní, nemělo by se to dít, ale je třeba s tím počítat. (A takový workspace ignorovat.)

jprochazk · 2022-10-10T08:53:37Z

To by se mělo ignorovat už v go-client. Můžeš mě pozvat do toho projektu?

JakubMatejka · 2022-10-10T08:59:06Z

@jprochazk aha, jasně. Je to https://connection.north-europe.azure.keboola.com/admin/projects/1268 přístup tam už máš

jprochazk · 2022-10-10T09:00:58Z

V go-client se kontroluje jen config bez sandboxId, ale když tam je sandboxId a ten sandbox neexistuje, tak to pak už nefunguje 🙂.

jprochazk · 2022-10-10T09:07:12Z

keboola/go-client#31

jprochazk · 2022-10-10T09:51:30Z

@jprochazk aha, jasně. Je to https://connection.north-europe.azure.keboola.com/admin/projects/1268 přístup tam už máš

Po upgrade mám tenhle output, funguje ti to už?

$ ./target/keboola-cli_linux_amd64/kbc remote workspace list
Loaded env file ".env".
Storage API host "connection.north-europe.azure.keboola.com" set from ENV.
Storage API token set from ENV.
Loading workspaces, please wait.
Found workspaces:
  ID: 1699268, Type: python, Size: small, Name: test
  ID: 4429297, Type: snowflake, Name: test-snowflake
  ID: 731093, Type: python-mlflow, Name: register model

JakubMatejka · 2022-10-10T10:02:17Z

Funguje. Ale jak teď na to koukám, tak bych asi spíš ty jednotlivý properties odřádkoval 😄 imho už jich je tam s tou size na jednom řádku nějak moc (a jednotlivý workspacy oddělil prázdným řádkem), obdobně jako list templates ať je to konzistentní: https://developers.keboola.com/cli/commands/template/list/

jprochazk · 2022-10-10T10:15:28Z

Nesouhlasím s tím, že by tam toho bylo moc. U templates mi zase přijde že je tam zbytečně moc "dead space". V malým terminal window se mi tam nevleze ani jeden celý template. Jakmile máš 3+ tak už je to docela extrém, protože to nemá delimiters tak je špatně vidět, kde končí jeden template a začíná další. Určitě bysme to měli nějak standardizovat, ale chtěl bych to takhle zatím nechat a možná se k tomu ještě vrátit někdy jindy.

Co si o tom myslí @michaljurecko?

michaljurecko · 2022-10-10T10:33:14Z

🙂 ja som nad tym tiez rozmyslal, ale neprisiel som na jednoznacne riesenie:

napr. nemal by byt name prvy? ked bude mat user 20x workspace, bude sa v tom schopny zorientovat (?)
ale mozno to nie je nieco co my riesit STDOUT, ale skor nejaky "lepsi" interface, kde sa da aj vyhladavat (alebo user pouzije nejaky iny CLI tool a tam presmeruje vystup)
takze neviem, osobne mi pride vystup pri templates prehladnejsi, ... delimiter sa tam moze pridat .... nevadi mi ani, ked by som si to v pripade potreby presmeroval do suboru a vyhladaval v tom cez VSCode
takze som vam velmi nepomohol 😃
(mozete dat 2 screenshoty do slacku a zistit co si o tom myslia ludia, alebo to neriesit)

michaljurecko · 2022-10-10T10:38:22Z

Ako to riesi napr. Kubernetes CLI?

jprochazk · 2022-10-10T10:42:34Z

Takový "compact table" interface:

michaljurecko · 2022-10-10T10:46:36Z

Pri workspace by sa to dalo, pri templates uz nie + name je prvy.

michaljurecko · 2022-10-10T10:47:03Z

Ale mozeme to vyriesit aj neskor a dat do backlogu, a pozriet sa aj na ostatne prikazy.

jprochazk · 2022-10-10T11:03:54Z

Jen nechci zůstat tady zaseklý, tenhle PR obsahuje víc než jen workspace list implementaci, takže to blokuje další věci. Byl bych pro to dát do backlogu, není potřeba to řešit hned.

JakubMatejka · 2022-10-10T11:14:49Z

No jak jsem psal, tak je to takhle imho nepřehledný a k tomu jsem velký zastánce konzistentnosti. Pokud se na tom ale shodneme, tak to nechme a řešme později. 🤷‍♂️

Tak co třeba dát první název jak píše Michal a ostatní parametry trochu upozadit, třeba oddělit závorkama?

test (ID: 1699268, Type: python, Size: small)
test-snowflake (ID: 4429297, Type: snowflake)
register model (ID: 731093, Type: python-mlflow)

jprochazk · 2022-10-10T11:35:45Z

Pls teď se o tom nebavme, jak říká Michal, je asi potřeba vymyslet lepší interface pomocí kterýho to budeme vypisovat, podobně jako máme interface na selecty 🙂. V tom interface můžeme standardizovat indentation, delimiters, jak vypisovat key-value pairs, atd. Můžeme pak mít flagy pro změnu outputu, např. renderovat jako "pretty table" a nebo renderovat jako CSV pro processing v jiném commandu. Ale hlavně to bude pak konzistentní. Vytvořil jsem na to epic v maintenance https://keboola.atlassian.net/browse/KAC-268 (nevěděl jsem, kam jinam zařadit). Je to tak OK?

JakubMatejka · 2022-10-10T11:46:14Z

Ano, souhlasím, koncepčně to řešme později. Ale říkám že než to tak uděláme, tak aspoň trochu zpřehledněme ten output. Nebo to je problém?

JakubMatejka · 2022-10-10T11:53:55Z

Vždycky mě můžete overridenout, blokovat to zbytečně nechci. Jen nejsem úplně fanda konceptu "rychle mergovat, fixovat později", to pak nemusím dělat review. 🙂

jprochazk · 2022-10-10T11:54:24Z

Je to problém, protože by bylo potřeba se shodnout na tom, co je "přehlednější", což je subjektivní věc. Budeme se točit pořád dokola a nebude to produktivní. Ta implementace je ale hotová, output minimálně obsahuje všechny důležité informace a hlavně tohle blokuje další věci (což je moje chyba, měl jsem ty "unrelated" změny házet do jiných PR).

JakubMatejka · 2022-10-10T11:59:44Z

Ve chvílích kdy je to subjektivní dávám přednost konzistenci a názoru vývojáře který za tím kódem stojí nejdýl, tak ať to rozhodne Michal. 😄

jprochazk · 2022-10-10T12:21:58Z

Tak co třeba dát první název jak píše Michal a ostatní parametry trochu upozadit, třeba oddělit závorkama

Stačí to tak?

michaljurecko · 2022-10-10T12:34:54Z

Mne staci, ze bude ten name prvy 😉

JakubMatejka · 2022-10-10T12:36:55Z

Jo jo, taky tak. 🙂 👍

JakubMatejka

lgtm

jprochazk requested review from JakubMatejka and michaljurecko September 27, 2022 15:28

jprochazk force-pushed the janp-KAC-232-list-workspaces branch from e4792a9 to 7b7414d Compare September 28, 2022 10:16

michaljurecko reviewed Sep 29, 2022

View reviewed changes

pkg/lib/operation/project/remote/workspace/list.go Outdated Show resolved Hide resolved

pkg/lib/operation/project/remote/workspace/list.go Outdated Show resolved Hide resolved

pkg/lib/operation/project/remote/workspace/list.go Outdated Show resolved Hide resolved

jprochazk force-pushed the janp-KAC-232-list-workspaces branch from 2a00f7d to 62c832e Compare October 7, 2022 09:27

jprochazk requested a review from michaljurecko October 7, 2022 09:34

jprochazk force-pushed the janp-KAC-232-list-workspaces branch from a1ad314 to a01e60d Compare October 7, 2022 12:33

jprochazk added 2 commits October 10, 2022 11:34

upgrade go-client to v0.8.1

615abdf

fix go.sum

e659623

JakubMatejka self-requested a review October 10, 2022 09:57

update stdout format

5114783

JakubMatejka approved these changes Oct 10, 2022

View reviewed changes

jprochazk merged commit 578dccb into main Oct 10, 2022

jprochazk deleted the janp-KAC-232-list-workspaces branch October 10, 2022 12:39

List Workspaces CLI command #841

List Workspaces CLI command #841

Conversation

jprochazk commented Sep 27, 2022

jprochazk commented Sep 27, 2022

michaljurecko commented Sep 27, 2022

michaljurecko commented Sep 27, 2022 • edited Loading

jprochazk commented Sep 28, 2022

jprochazk commented Oct 7, 2022

jprochazk commented Oct 7, 2022

jprochazk commented Oct 7, 2022

JakubMatejka commented Oct 7, 2022

JakubMatejka commented Oct 7, 2022

JakubMatejka commented Oct 7, 2022

JakubMatejka commented Oct 7, 2022

jprochazk commented Oct 7, 2022

michaljurecko commented Oct 7, 2022 • edited Loading

jprochazk commented Oct 7, 2022 • edited Loading

JakubMatejka commented Oct 7, 2022

jprochazk commented Oct 7, 2022 • edited Loading

jprochazk commented Oct 7, 2022

michaljurecko commented Oct 7, 2022

JakubMatejka commented Oct 7, 2022

jprochazk commented Oct 7, 2022

michaljurecko commented Oct 7, 2022

jprochazk commented Oct 7, 2022

jprochazk commented Oct 7, 2022

JakubMatejka commented Oct 10, 2022

jprochazk commented Oct 10, 2022

JakubMatejka commented Oct 10, 2022

jprochazk commented Oct 10, 2022

jprochazk commented Oct 10, 2022

jprochazk commented Oct 10, 2022

JakubMatejka commented Oct 10, 2022

jprochazk commented Oct 10, 2022 • edited Loading

michaljurecko commented Oct 10, 2022 • edited Loading

michaljurecko commented Oct 10, 2022

jprochazk commented Oct 10, 2022

michaljurecko commented Oct 10, 2022

michaljurecko commented Oct 10, 2022

jprochazk commented Oct 10, 2022

JakubMatejka commented Oct 10, 2022

jprochazk commented Oct 10, 2022

JakubMatejka commented Oct 10, 2022

JakubMatejka commented Oct 10, 2022

jprochazk commented Oct 10, 2022 • edited Loading

JakubMatejka commented Oct 10, 2022

jprochazk commented Oct 10, 2022

michaljurecko commented Oct 10, 2022

JakubMatejka commented Oct 10, 2022

JakubMatejka left a comment

Choose a reason for hiding this comment

michaljurecko commented Sep 27, 2022 •

edited

Loading

michaljurecko commented Oct 7, 2022 •

edited

Loading

jprochazk commented Oct 7, 2022 •

edited

Loading

jprochazk commented Oct 7, 2022 •

edited

Loading

jprochazk commented Oct 10, 2022 •

edited

Loading

michaljurecko commented Oct 10, 2022 •

edited

Loading

jprochazk commented Oct 10, 2022 •

edited

Loading