Add show latest command that prints latest version that conform to contraints #536

MatthewJohn · 2025-01-24T08:01:40Z

Issue #301

…n earlier return

yermulnik

Should we deem feat: allow to show required by module version explicitly #301 obsolete and close in favour of this PR? (I'm closing that PR for now)
Do we need any tests for this new arg?
- On a side note, we probably may need to look into restraining simultaneous use of cmdline args that are not compatible with each other (I mean this is a thought aloud)
I added commit suggestions to try and align this new arg with what it actually does (e.g. when version is provided via command line, this new arg does not show latest required, but outputs version requested on command line. same for version parameter in TOML. so on by means of all of the ifs from lib/param_parsing/parameters.go). WDYT?
Let me know if you'd like me to postpone new version release for --arch arg to get this PR added to the same release (or to cut next release for it later)

yermulnik · 2025-01-24T13:48:22Z

lib/param_parsing/parameters.go

+	MirrorURL              string
+	ShowLatestFlag         bool
+	ShowLatestPre          string
+	ShowLatestRequiredFlag bool


The Flag suffix looks pointless to me. Not sure what its value is and not sure why it was added to a few others in past. I guess the only exception are Version and VersionFlag (where both could have a more clear names though).
Would you mind removing this suffix so this new flag is in consistency with similar ones nearby?
Thanks.

Suggested change

ShowLatestRequiredFlag bool

ShowRequired bool

yermulnik · 2025-01-24T13:50:52Z

lib/param_parsing/parameters.go

@@ -66,6 +67,7 @@ func populateParams(params Params) Params {
 	getopt.StringVarLong(&params.LogLevel, "log-level", 'g', "Set loglevel for tfswitch. One of (ERROR, INFO, NOTICE, DEBUG, TRACE)")
 	getopt.StringVarLong(&params.MirrorURL, "mirror", 'm', "install from a remote API other than the default. Default (based on product):\n"+strings.Join(defaultMirrors, "\n"))
 	getopt.BoolVarLong(&params.ShowLatestFlag, "show-latest", 'U', "Show latest stable version")
+	getopt.BoolVarLong(&params.ShowLatestRequiredFlag, "show-latest-required", 'R', "Show latest stable version, which complies to constraints set by Terraform/Terragrunt")


Terraform/Terragrunt and OpenTofu and by other allowed means like TOML file =)

Suggested change

getopt.BoolVarLong(&params.ShowLatestRequiredFlag, "show-latest-required", 'R', "Show latest stable version, which complies to constraints set by Terraform/Terragrunt")

getopt.BoolVarLong(&params.ShowRequired, "show-required", 'R', "Show version, which complies to version requested by configuration")

yermulnik · 2025-01-24T14:26:27Z

lib/param_parsing/parameters.go

@@ -177,6 +179,7 @@ func initParams(params Params) Params {
 	params.MirrorURL = ""
 	params.ShowLatestFlag = false
 	params.ShowLatestPre = lib.DefaultLatest
+	params.ShowLatestRequiredFlag = false


Suggested change

params.ShowLatestRequiredFlag = false

params.ShowRequired = false

yermulnik · 2025-01-24T14:26:49Z

main.go

+	case parameters.ShowLatestRequiredFlag:
+		/* show latest stable version within constraints */
+		lib.ShowLatestRequiredVersion(parameters.MirrorURL, parameters.Version)


Suggested change

case parameters.ShowLatestRequiredFlag:

/* show latest stable version within constraints */

lib.ShowLatestRequiredVersion(parameters.MirrorURL, parameters.Version)

case parameters.ShowRequired:

/* show version requested by configuration */

lib.ShowRequiredVersion(parameters.MirrorURL, parameters.Version)

yermulnik · 2025-01-24T14:28:07Z

lib/list_versions.go

+// ShowLatestRequiredVersion show latest version using constraints from TF
+func ShowLatestRequiredVersion(mirrorURL string, version string) {


Suggested change

// ShowLatestRequiredVersion show latest version using constraints from TF

func ShowLatestRequiredVersion(mirrorURL string, version string) {

// ShowRequiredVersion show latest version using constraints from TF

func ShowRequiredVersion(mirrorURL string, version string) {

yermulnik · 2025-01-24T14:29:16Z

lib/list_versions.go

+		fmt.Printf("%s\n", version)
+		return
+	}
+	logger.Fatal("The provided terraform version does not exist.")


Suggested change

logger.Fatal("The provided terraform version does not exist.")

logger.Fatalf("The requested version (%q) does not exist.", version)

warrensbox · 2025-01-25T08:16:37Z

openAI

Should we deem feat: allow to show required by module version explicitly #301 obsolete and close in favour of this PR? (I'm closing that PR for now)

Do we need any tests for this new arg?

On a side note, we probably may need to look into restraining simultaneous use of cmdline args that are not compatible with each other (I mean this is a thought aloud)

I added commit suggestions to try and align this new arg with what it actually does (e.g. when version is provided via command line, this new arg does not show latest required, but outputs version requested on command line. same for version parameter in TOML. so on by means of all of the ifs from lib/param_parsing/parameters.go). WDYT?

Let me know if you'd like me to postpone new version release for --arch arg to get this PR added to the same release (or to cut next release for it later)

Do we need any tests for this new arg?

This would be ideal

On a side note, we probably may need to look into restraining simultaneous use of cmdline args that are not compatible with each other

I think we have had checks in the past to check this. It's worth checking this again
fair commit suggestions

Let me know if you'd like me to postpone new version release for --arch arg to get this PR added to the same release

I suggest 2 different releases, incase we need to roll back any of this new features

@yermulnik feel free to do the deployments for the release

MatthewJohn · 2025-01-26T09:12:42Z

Do we need any tests for this new arg?

Given the argument just prints to stdout and we don't have any mocking, I wonder if updating the function to return the string that is displayed to the user back to main.go might make this a lot easier? Without it, I'm not sure the easiest way to do it in golang.

On a side note, we probably may need to look into restraining simultaneous use of cmdline args that are not compatible with each other

The other argparsers I've seen have got this kind of functionality, but can't see viper does - assume it's something you had to do manually in the past?

I suggest 2 different releases, incase we need to roll back any of this new features

👍

I added commit suggestions to try and align this new arg...

Yes, I agree none of it makes much sense given it's high dependency on the argument parsing - it's somewhat of a shame that this is all done magically behind before the main case statement. Happy with any name, since it's just printing a pre-defined version argument.
Given all this, I can't see that the function is of much use at all as a third-party library (I guess, ideally it could be private).
If we do think it's completely unusued publicly, then maybe merging the function with ShowLatestVersion would make sense?

yermulnik · 2025-01-26T18:41:01Z

If we do think it's completely unusued publicly, then maybe merging the function with ShowLatestVersion would make sense?

I like the ShowRequired/ShowRequiredVersion naming as it reflects what this feature outputs. WDYT?

yermulnik · 2025-01-26T18:55:14Z

I suggest 2 different releases, incase we need to roll back any of this new features

@yermulnik feel free to do the deployments for the release

@warrensbox @MatthewJohn
I went with minor version bump: v1.3.0
Please review #539 and I'll cut v1.3.0 release once approved.

yermulnik · 2025-02-05T12:50:20Z

@MatthewJohn Will you be giving this PR some love? =)

MatthewJohn · 2025-02-05T17:09:37Z

Hey sorry, Had a little birth in the family - will take a look in tje next 24h when I’m next awake :)

…

Sent from my iPhone

On 5 Feb 2025, at 12:50, George L. Yermulnik ***@***.***> wrote: @MatthewJohn Will you be giving this PR some love? =) — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

yermulnik · 2025-02-05T17:18:18Z

Hey sorry, Had a little birth in the family - will take a look in tje next 24h when I’m next awake :)

Ah, congrats 🥳 Take your time and take some sleep =)))

MatthewJohn added 3 commits January 24, 2025 08:00

Add support for showing the latest version by constraint

74eca56

Issue #301

Correct typo in comment

8c2a799

Remove comparison of err, since if err is not null, it is caught by a…

ee63a78

…n earlier return

MatthewJohn requested review from warrensbox, yermulnik, jukie, MatrixCrawler and crablab as code owners January 24, 2025 08:01

yermulnik reviewed Jan 24, 2025

View reviewed changes

yermulnik mentioned this pull request Jan 24, 2025

feat: allow to show required by module version explicitly #301

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add show latest command that prints latest version that conform to contraints #536

Add show latest command that prints latest version that conform to contraints #536

MatthewJohn commented Jan 24, 2025 •

edited

Loading

yermulnik left a comment •

edited

Loading

yermulnik Jan 24, 2025

yermulnik Jan 24, 2025 •

edited

Loading

yermulnik Jan 24, 2025 •

edited

Loading

yermulnik Jan 24, 2025 •

edited

Loading

yermulnik Jan 24, 2025 •

edited

Loading

yermulnik Jan 24, 2025

yermulnik Jan 24, 2025

warrensbox commented Jan 25, 2025 •

edited

Loading

MatthewJohn commented Jan 26, 2025

yermulnik commented Jan 26, 2025

yermulnik commented Jan 26, 2025

yermulnik commented Feb 5, 2025

MatthewJohn commented Feb 5, 2025 via email

yermulnik commented Feb 5, 2025

	getopt.BoolVarLong(&params.ShowLatestRequiredFlag, "show-latest-required", 'R', "Show latest stable version, which complies to constraints set by Terraform/Terragrunt")
	getopt.BoolVarLong(&params.ShowRequired, "show-required", 'R', "Show version, which complies to version requested by configuration")

	params.ShowLatestRequiredFlag = false
	params.ShowRequired = false

		// ShowLatestRequiredVersion show latest version using constraints from TF
		func ShowLatestRequiredVersion(mirrorURL string, version string) {

	logger.Fatal("The provided terraform version does not exist.")
	logger.Fatalf("The requested version (%q) does not exist.", version)

Add show latest command that prints latest version that conform to contraints #536

Are you sure you want to change the base?

Add show latest command that prints latest version that conform to contraints #536

Conversation

MatthewJohn commented Jan 24, 2025 • edited Loading

yermulnik left a comment • edited Loading

Choose a reason for hiding this comment

yermulnik Jan 24, 2025

Choose a reason for hiding this comment

yermulnik Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

yermulnik Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

yermulnik Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

yermulnik Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

yermulnik Jan 24, 2025

Choose a reason for hiding this comment

yermulnik Jan 24, 2025

Choose a reason for hiding this comment

warrensbox commented Jan 25, 2025 • edited Loading

MatthewJohn commented Jan 26, 2025

yermulnik commented Jan 26, 2025

yermulnik commented Jan 26, 2025

yermulnik commented Feb 5, 2025

MatthewJohn commented Feb 5, 2025 via email

yermulnik commented Feb 5, 2025

MatthewJohn commented Jan 24, 2025 •

edited

Loading

yermulnik left a comment •

edited

Loading

yermulnik Jan 24, 2025 •

edited

Loading

yermulnik Jan 24, 2025 •

edited

Loading

yermulnik Jan 24, 2025 •

edited

Loading

yermulnik Jan 24, 2025 •

edited

Loading

warrensbox commented Jan 25, 2025 •

edited

Loading