compose-spec / compose-go

Reference library for parsing and loading Compose YAML files
https://compose-spec.io
Apache License 2.0
355 stars 112 forks source link

Version warning message causing lots of churn amongst projects with minimal benefit #641

Closed glenjamin closed 3 months ago

glenjamin commented 4 months ago

I see https://github.com/compose-spec/compose-go/pull/640 was just closed due to lack of a raised issue as I was typing this comment there, so I'm raising the issue for discussion first.

There is a bunch of discussion about the impact of the warning and the questionable benefits in https://github.com/docker/compose/issues/11628. It's likely that the majority of users of docker-compose are not aware that the compose spec is a separate project - I wasn't until just now when I went to look at the history of this change.

I can see that the warning was first introduced in https://github.com/compose-spec/compose-go/pull/575 and later added to the spec in https://github.com/compose-spec/compose-spec/pull/489 - but it's unclear from those PRs how much discussion was had in other forums and whether the cost of creating warnings outweights the benefits vs simply ignoring the field.

These comments summarise the cost of introducing the warning and showing it to loads of people: https://github.com/docker/compose/issues/11628#issuecomment-2061299025 https://github.com/docker/compose/issues/11628#issuecomment-2176213424 https://github.com/docker/compose/issues/11628#issuecomment-2176316608

Are there large costs to retaining the version field but ignoring it that I've overlooked?

rseymour commented 4 months ago

From a cursory grep through the code, it's as if the only log level used is Warn. But logrus supports everything from Trace to Fatal.

That said, I think the only way through this is perhaps to change the log message from:

`version` is obsolete

to something like

the key `version` in %s is deprecated and is ignored

At least that way folks wouldn't think the version they are using is obsolete.

glours commented 4 months ago

Hey @glenjamin @rseymour I'm fine to change the message to make it clearer but I also want to let you know we won't go back to a info message log. As I already explained in #640 a lot of users struggle with the version, it confuses the communication with them when they open issues, just see this one opened yesterday where the user think he's using Compose v3 when the latest version is v2.27.1 (This is just one example among many others)

So I can understand your frustration about the fact you need to update multiple Compose files to avoid seeing this warning on your side. But if you see this from our perspective we need to constantly:

glenjamin commented 4 months ago

Thank you for the clear explanation @glours !

In particular bug example since the user provided both their compose file and the --version output then it's arguable whether the confusion is significant, but I can certainly see how it could lead to harder triage in general.

Perhaps the message could be along the lines of

`version` is ignored and should be removed to avoid potential confusion

And then it's clear what action to take without needing to do additional research?