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

Added build-catalog-tool-timeout parameter to install command. #4821

Closed
wants to merge 3 commits into from

Conversation

valdar
Copy link
Member

@valdar valdar commented Oct 12, 2023

Added build-catalog-tool-timeout parameter to the install command.
Added CAMEL_K_BUILD_CATALOG_TOOL_TIMEOUT to e2e tests scripts to leverage the new install parameter.
Updated doc.
Small addition to .gitignore

This is useful in environment where the default timeout of 2 minutes is not sufficient.

Release Note

NONE

Added CAMEL_K_BUILD_CATALOG_TOOL_TIMEOUT to e2e tests to leverage the new install parameter.
Updated doc.
@github-actions
Copy link
Contributor

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage unchanged.

Copy link
Contributor

@gansheer gansheer left a comment

Choose a reason for hiding this comment

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

I might be missing something, but I am not sure this is a good idea to add a flag on a deprecated field :

// the timeout (in seconds) to use when creating the build tools container image
// Deprecated: no longer in use
BuildCatalogToolTimeout *metav1.Duration `json:"buildCatalogToolTimeout,omitempty"`

docs/modules/ROOT/pages/contributing/e2e.adoc Outdated Show resolved Hide resolved
Copy link
Contributor

@squakez squakez left a comment

Choose a reason for hiding this comment

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

The field is deprecated. It is already not used in version 2.1 where we got rid off dynamic builder catalog.

@oscerd
Copy link
Contributor

oscerd commented Oct 13, 2023

If 2.0.x is going to have patch releases, we need to merge this one.

@squakez
Copy link
Contributor

squakez commented Oct 13, 2023

If 2.0.x is going to have patch releases, we need to merge this one.

In case we should do on release-2.x branch only.

@oscerd
Copy link
Contributor

oscerd commented Oct 13, 2023

We are still publishing the nightly for 2.0.2

@oscerd
Copy link
Contributor

oscerd commented Oct 13, 2023

If 2.0.x is not going to be released anymore, we should avoid publishing nightly. This PR is not dangerous and I think the target is the nightly release, so we could merge and in case there will be any future release it will be there, in case not, there is always the nightly.

@squakez
Copy link
Contributor

squakez commented Oct 13, 2023

If 2.0.x is not going to be released anymore, we should avoid publishing nightly. This PR is not dangerous and I think the target is the nightly release, so we could merge and in case there will be any future release it will be there, in case not, there is always the nightly.

Yes. The target of this PR is main. If we want it on release branch it should be created another PR targeting it.

@oscerd
Copy link
Contributor

oscerd commented Oct 13, 2023

If 2.0.x is not going to be released anymore, we should avoid publishing nightly. This PR is not dangerous and I think the target is the nightly release, so we could merge and in case there will be any future release it will be there, in case not, there is always the nightly.

Yes. The target of this PR is main. If we want it on release branch it should be created another PR targeting it.

Ah, maybe I should check the branch before commenting nonsense!

@github-actions
Copy link
Contributor

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage unchanged.

1 similar comment
@github-actions
Copy link
Contributor

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage unchanged.

@valdar
Copy link
Member Author

valdar commented Oct 16, 2023

Ok thank to @squakez I figured out that for some days this PR is actually irrelevant for main due to #4764 .
I have ported it to release-2.0.x and opened against that branch here #4827 if it can be useful.

@valdar valdar closed this Oct 16, 2023
@valdar valdar deleted the e2eTestImpr branch October 16, 2023 17:03
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.

4 participants