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.

Sign up for GitHub

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

Jump to bottom

Make podman-compose refer to podman-compose(1) when using an external provider #23074

Merged
merged 1 commit into from
Jul 12, 2024
Merged

Make podman-compose refer to podman-compose(1) when using an external provider #23074

merged 1 commit into from
Jul 12, 2024

Conversation

marinmo
Copy link
Contributor

@marinmo marinmo commented Jun 23, 2024 •
edited by Luap99
Loading

This PR is meant to fix #22904. It makes the warning message emitted by podman-compose when using an external provider point to podman-compose(1) which is also updated to include instructions on how to disable said message.

Does this PR introduce a user-facing change?

Changed the warning message emitted by `podman-compose` when using an external compose provider. Instead of referring to "the documentation" it now refers to the man page of `podman-compose(1)` which is also updated to contain the relevant information.

@openshift-ci openshift-ci bot added the do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None label Jun 23, 2024
@marinmo
Copy link
Contributor Author

marinmo commented Jun 23, 2024 •
edited
Loading

Seems the build is failing due to tests which really shouldn't be necessary for a man page? Please lmk if any further action is required on my part.

rhatdan
rhatdan reviewed Jun 24, 2024
View reviewed changes
docs/source/markdown/podman-compose.1.md.in Outdated
@@ -13,6 +13,8 @@ The default compose providers are `docker-compose` and `podman-compose`. If ins

If you want to change the default behavior or have a custom installation path for your provider of choice, please change the `compose_provider` field in `containers.conf(5)`. You may also set the `PODMAN_COMPOSE_PROVIDER` environment variable.

By default, `podman-compose` will emit a warning when using an external compose provider (such as `docker-compose`). This warning can be disabled by setting `compose_warning_logs` to false in `containers.conf(5)` or setting the `PODMAN_COMPOSE_WARNING_LOGS` environment variable to false. See the man page for `containers.conf(5)` for more information.
Copy link
Member

Choose a reason for hiding this comment

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

I believe this should be podman compose podman-compose and docker-compose are both two external commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Example given in #22904 is /usr/local/bin/docker-compose which is why I used that example. Having podman-compose being referred to as "External compose provider" when invoking podman compose seems ... Confusing, at best, to me - but I'm open to change :) Could it be more vendor neutral? Perhaps something like
"By default, podman compose will emit a warning when using an external compose provider. [...]"?

Luap99
Luap99 reviewed Jul 8, 2024
View reviewed changes
docs/source/markdown/podman-compose.1.md.in Outdated Show resolved Hide resolved
cmd/podman/compose.go Show resolved Hide resolved
@Luap99
Copy link
Member

Luap99 commented Jul 8, 2024

Also please squash the commits and rebase

@marinmo marinmo force-pushed the podman-compose-providerfix branch from d667f1e to 701c54f Compare July 8, 2024 16:25
@Luap99 Luap99 mentioned this pull request Jul 10, 2024
Closed
@Luap99
Copy link
Member

Luap99 commented Jul 10, 2024

@marinmo Can you please squash the commits (and fix the missing sign-of line with that)
And make sure you rebase on latest main please.

@marinmo marinmo force-pushed the podman-compose-providerfix branch from 701c54f to 3e38027 Compare July 11, 2024 18:02
@marinmo
Copy link
Contributor Author

marinmo commented Jul 11, 2024

Think that was a rather crude fix on my side, my git skills are lacking unfortunately. Squashed but accidentally left both comments intact. Rebased main (albeit after force pushing - does it matter?).

@rhatdan rhatdan added the No New Tests Allow PR to proceed without adding regression tests label Jul 11, 2024
@rhatdan
Copy link
Member

rhatdan commented Jul 11, 2024

git commit -a --amend -s
git push --force

@marinmo
Update warning message when using external compose provider
02af4f8 
Change the warning message at runtime to refer to the man page of podman-compose instead of "the documentation"

Add instructions in the man page on how to disable the warning emitted by podman-compose when using an external compose provider

Signed-off-by: marinmo <bugzilla@marinmo.org>
@marinmo marinmo force-pushed the podman-compose-providerfix branch from 3e38027 to 02af4f8 Compare July 12, 2024 05:14
@openshift-ci openshift-ci bot added release-note and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Jul 12, 2024
Luap99
Luap99 approved these changes Jul 12, 2024
View reviewed changes
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Jul 12, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, marinmo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 12, 2024
@Luap99
Copy link
Member

Luap99 commented Jul 12, 2024

Thank you.

@rhatdan
Copy link
Member

rhatdan commented Jul 12, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 12, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit e62c928 into containers:main Jul 12, 2024
90 checks passed
@edsantiago edsantiago mentioned this pull request Jul 24, 2024
Closed
@legobeat legobeat mentioned this pull request Jul 30, 2024
Merged
legobeat
legobeat reviewed Jul 30, 2024
View reviewed changes
docs/source/markdown/podman-compose.1.md.in
@@ -13,6 +13,8 @@ The default compose providers are `docker-compose` and `podman-compose`. If ins

If you want to change the default behavior or have a custom installation path for your provider of choice, please change the `compose_provider` field in `containers.conf(5)`. You may also set the `PODMAN_COMPOSE_PROVIDER` environment variable.

By default, `podman compose` will emit a warning saying that it executes an external command. This warning can be disabled by setting `compose_warning_logs` to false in `containers.conf(5)` or setting the `PODMAN_COMPOSE_WARNING_LOGS` environment variable to false. See the man page for `containers.conf(5)` for more information.
Copy link

Choose a reason for hiding this comment

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

It seems that the PODMAN_COMPOSE_WARNING_LOGS is not actually handled?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhatdan rhatdan rhatdan left review comments

@legobeat legobeat legobeat left review comments

@Luap99 Luap99 Luap99 approved these changes

Assignees

@rhatdan rhatdan

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. No New Tests Allow PR to proceed without adding regression tests release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow disabling "Executing external compose provider"
4 participants
@marinmo @Luap99 @rhatdan @legobeat