Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kube refine for backup #793

Merged
merged 16 commits into from
Mar 30, 2024
Merged

kube refine for backup #793

merged 16 commits into from
Mar 30, 2024

Conversation

fj0r
Copy link
Contributor

@fj0r fj0r commented Mar 15, 2024

  • new function kube refine

    • define the required information in $env.KUBERNETES_REFINE, kube refine will collect information according to its definition
    • definitions in status and cluster_status are runtime information and will not be collected. They are used in the kg command to display status
    • normalize the output of the kg, kgp, kgs command using krefine
  • rename kcconf to kccc (kubectl change context clone)

  • a new module, refine.nu, declaratively extracts data from complex structures.

  • container-list additionally displays the cmd field of the image

Define the required information in `$env.KUBERNETES_REFINE`,
`kube refine` will collect information according to its definition

Definitions in `status` and `cluster_status` are runtime information and will not be collected. They are used in the `kg` command to display status

A new module, `refine.nu`, declaratively extracts data from complex structures.

`container-list` additionally displays the cmd field of the image

rename `kcconf` to `kccc` (kubectl change context clone)
- docker volume dump/restore
- host-path support volume
- container-remove safely
- image-list
    - parse created
    - fix id
- image-select
@fdncred
Copy link
Collaborator

fdncred commented Mar 29, 2024

I'm not a fan of these changes. It's hard to tell what this argx file you're putting in several folders does. There are changes not mentioned in the description. You're creating a new log command when there's already one in the std lib. Lack of comments explaining what the custom commands do.

@fdncred fdncred marked this pull request as draft March 29, 2024 11:48
@fj0r
Copy link
Contributor Author

fj0r commented Mar 29, 2024

The argx file is copied from modules/argx/argx.nu for the purpose of passing the ci test.I'm not sure if the CI test must be passed, and I don't know of any other better methods.

Log command can support structured logs in logfmt (not strict), and can output to a file (I'm not sure if std log can).
It has four parts:

  • time
  • level
  • all the records in the arguments are merged together as the logfmt part,
  • all strings are merged together as the plaintext part

The latter two are optional.
If called with log level <num>, the level name can be hidden, which is suitable for scenarios where you only want to distinguish by color.
There is also an additional trace level, level names are unified into three characters to keep them neat. Because inf literal are recognized as number, an additional synonym msg is added.
Use (U+2502) instead of (pipe) as the delimiter because the message may contain (pipe) and (U+2502) is easier to parse.
For me, the most important thing is carefully adjusted output, finer highlights(apply color only on datetime to improve readability), soft colors, and better ergonomics.
In my opinion, custom colors and formatting are not that important because they are not easy enough to take a few minutes to do. It may be easier to support customization. In practice, it is important to provide a good set of default configurations
202403292233_1024x374

This part was originally written directly as a function in kubernetes.nu. Later, there were too many references, so it was taken out as a separate module. If I follow the original method, I need to copy the kubernetes.nu file and then paste this section, which is too troublesome. I can also remove the part kube refine that refers to it, but then I need to delete this section every time I update it.

@fdncred
Copy link
Collaborator

fdncred commented Mar 30, 2024

Your changes are better but I'm still not a fan of log. I'd prefer it be a different name so it doesn't stomp on our log in std-lib.

`--multiline` control formatting
Remove redundant argx.nu
@fj0r
Copy link
Contributor Author

fj0r commented Mar 30, 2024

ok, I understand, how about changing it to slog or lg?

update: has been changed to lg

@fdncred
Copy link
Collaborator

fdncred commented Mar 30, 2024

ok, good enough. do you want to keep working on it or are you done?

@fj0r
Copy link
Contributor Author

fj0r commented Mar 30, 2024

88be461
I renamed some modules and it feels good (sorry, just found out about mod.nu)
But in this case, the user needs to make some adjustments. I don’t know if this is appropriate.

@fj0r
Copy link
Contributor Author

fj0r commented Mar 30, 2024

https://github.com/nushell/nu_scripts/actions/runs/8488088746/job/23256669801
There is no problem with this directory structure in my local machine. I don’t know why ci panic again.

@fj0r
Copy link
Contributor Author

fj0r commented Mar 30, 2024

If this ok, please merge it. Otherwise, I will revert 88be461 .

@fj0r fj0r force-pushed the main branch 2 times, most recently from ba82366 to 91bd7bc Compare March 30, 2024 09:06
@fj0r fj0r marked this pull request as ready for review March 30, 2024 09:36
@fdncred
Copy link
Collaborator

fdncred commented Mar 30, 2024

The only thing that bothers me is that you renamed git-v2.nu to mod.nu as if your git module owns the changes in that folder. It may be better to make a gitv2 folder to switch to mod.nu so the other files don't get lost.

@fj0r
Copy link
Contributor Author

fj0r commented Mar 30, 2024

Ok, but the file git/README.md also belongs to gitv2, and I moved it there too.

@fdncred fdncred merged commit f399769 into nushell:main Mar 30, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants