Skip to content

chore: Fix BuildContextCompressionLevel description, output the level#9688

Merged
katiexzhang merged 2 commits into
GoogleContainerTools:mainfrom
idsulik:chore-changes
Jan 29, 2025
Merged

chore: Fix BuildContextCompressionLevel description, output the level#9688
katiexzhang merged 2 commits into
GoogleContainerTools:mainfrom
idsulik:chore-changes

Conversation

@idsulik

@idsulik idsulik commented Jan 28, 2025

Copy link
Copy Markdown
Contributor

Description

  1. Fixed BuildContextCompressionLevel description
  2. Add logging for the compression level - using gzip compression level {level}
Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
@idsulik idsulik requested a review from a team as a code owner January 28, 2025 06:39
@idsulik idsulik requested a review from mattsanta January 28, 2025 06:39
@ghost

ghost commented Jan 28, 2025

Copy link
Copy Markdown

Gemini encountered an error creating the summary. You can try again by commenting @code-review-assist summarize.

@katiexzhang

Copy link
Copy Markdown
Contributor

Thanks for the change @idsulik, although since there is no explicit request for this, I don't think it's necessary.

@idsulik

idsulik commented Jan 29, 2025

Copy link
Copy Markdown
Contributor Author

@katiexzhang , what do you mean? the description isn't full, how does the user should know what value they can use and what does it mean?

@idsulik

idsulik commented Jan 29, 2025

Copy link
Copy Markdown
Contributor Author

gzip compression level for the build context.

https://skaffold.dev/docs/references/yaml/#build-artifacts-kaniko-buildContextCompressionLevel

who knows what compression levels gzip has? I didn't even know until I made this feature.
Originally, I wrote all the levels in the description just for this purpose, so that the documentation would show it and the developers would know what value they could substitute, but because of a bug in the documentation generation, not the full description was published, and I didn't notice it.

@katiexzhang katiexzhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are you getting these descriptions from, specifically the -2? If possible, could you use the description from public docs, like https://docs.python.org/3/library/gzip.html.

Also I think you accidentally removed the default value.

@idsulik

idsulik commented Jan 29, 2025

Copy link
Copy Markdown
Contributor Author
  1. from here https://pkg.go.dev/compress/flate#pkg-constants, and it's the strong reason to put all the values in the docs, there is no need to wait while someone explicitly asks to do so
  2. the changes contain default value as well
@idsulik idsulik requested a review from katiexzhang January 29, 2025 17:24
"type": "integer",
"description": "gzip compression level for the build context.",
"x-intellij-html-description": "gzip compression level for the build context.",
"default": "1"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line was removed: "default": "1"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, sorry, haven't noticed it. pushed fix

Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
@katiexzhang katiexzhang merged commit acad129 into GoogleContainerTools:main Jan 29, 2025
@idsulik idsulik deleted the chore-changes branch January 30, 2025 05:59
plumpy pushed a commit to plumpy/skaffold that referenced this pull request Feb 4, 2025
…GoogleContainerTools#9688)

* chore: Fix BuildContextCompressionLevel description, output the level

Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>

* fixes

Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>

---------

Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
@menahyouyeah menahyouyeah mentioned this pull request Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 participants