Modify optional flag to use generics

[Its-Maniaco]

Description

Fixes #11154

Checklist

• "Backport to:" labels have been added if this change should be back-ported
• Tests were added or are not required
• Did the new or modified tests pass consistently locally and on the CI
• Documentation was added or is not required

[vitess-bot]

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

• Ensure that the Pull Request has a descriptive title.
• Ensure there is a link to an issue (except for internal cleanup and flaky test fixes), new features should have an RFC that documents use cases and test cases.

Tests

• Bug fixes should have at least one unit or end-to-end test, enhancement and new features should have a sufficient number of tests.

Documentation

• Apply the release notes (needs details) label if users need to know about this change.
• New features should be documented.
• There should be some code comments as to why things are implemented the way they are.
• There should be a comment at the top of each new or modified test to explain what the test does.

New flags

• Is this flag really necessary?
• Flag names must be clear and intuitive, use dashes (-), and have a clear help text.

If a workflow is added or modified:

• Each item in Jobs should be named in order to mark it as required.
• If the workflow needs to be marked as required, the maintainer team must be notified.

Backward compatibility

• Protobuf changes should be wire-compatible.
• Changes to _vt tables and RPCs need to be backward compatible.
• RPC changes should be compatible with vitess-operator
• If a flag is removed, then it should also be removed from vitess-operator and arewefastyet, if used there.
• vtctl command output order should be stable and awk-able.

[frouioui]

Hello @Its-Maniaco thank you for this first contribution! The DCO check is failing, can you please fix it using:

git rebase HEAD~1 --signoff
git push --force-with-lease origin optionalflag

From https://github.com/vitessio/vitess/pull/14645/checks?check_run_id=19177278801

[ajm188]

I think this should just be

Suggested change

	type OptionalType struct {
	val any
	set bool
	}
	type Optional[T any] struct {
	val T
	set bool
	}

and then rewriting the rest to match this type definition

[Its-Maniaco]

func (t OptionalType[any]) Set(arg string) error {
	tType := t.String()
	switch tType {
	case "float64":
		var temp interface{}
		temp, err := strconv.ParseFloat(arg, 64)
		if err != nil {
			return numError(err)
		}
		t.val = temp.(any)
	case "string":
		var temp interface{}
		temp = arg
		t.val = temp.(any)
	}
	t.set = true
	return nil
}

I was having trouble directly setting t.val because of type mismatch. So I came up with above workaround.
Please let me know if this is the correct approach.

[ajm188]

it's on the right track. i think you should be able to do just this, for example:

switch tType {
case "float64":
    temp, err := strconv.ParseFloat(...)
    if err != nil { ... }
    t.val = temp.(any)

[Its-Maniaco]

I tried that earlier but was given InvalidAssert.
invalid operation: temp (variable of type float64) is not an interface
Same error when I did:
t.val = arg.(any)

[ajm188]

ah then i guess you still need the declaration (but you should be able to declare as any rather than the older interface{})

[ajm188]

https://go.dev/play/p/16iecA5dkc3

[Its-Maniaco]

Sorry but I could not figure out what is wrong. Below is a screenshot.

[ajm188]

i don't think from the code you've shown that you have the right definition for Optional. does it match what i suggested here?

[Its-Maniaco]

Yeah I already rewrote it based on your suggestion.

[ajm188]

this approach seems to work: https://go.dev/play/p/RrQxZeRXD2a

[github-actions]

This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:

• Push additional commits to the associated branch.
• Remove the stale label.
• Add a comment indicating why it is not stale.

If no action is taken within 7 days, this PR will be closed.

[github-actions]

This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:

• Push additional commits to the associated branch.
• Remove the stale label.
• Add a comment indicating why it is not stale.

If no action is taken within 7 days, this PR will be closed.

[github-actions]

This PR was closed because it has been stale for 7 days with no activity.