Auto Versioning - Python-SDK

[grutt]

@abelanger5 I'm trying to wrap my head around this code as I port to TS.

# /hatchet/python-client/hatchet_sdk/clients/admin.py

# Determine if we should put the workflow
should_put = False
if auto_version:
    if workflow.version == "":
        workflow.version = "v0.1.0"
        should_put = True
    elif existing_workflow and existing_workflow.versions:
        if auto_version:
            workflow.version = bump_minor_version(existing_workflow.versions[0].version)
            should_put = True
        elif existing_workflow.versions[0].version != workflow.version:
            should_put = True
        else:
            should_put = False
    else:
        should_put = True

It looks like there are a few unreachable branches since we're checking auto_version at the beginning.

Thoughts on rewriting this using a guard pattern for legibility? I believe this is written with the same functionality as the original code:

def determine_workflow_update(auto_version, workflow, existing_workflow):
    if not auto_version:
        return False, workflow.version
    if workflow.version == "":
        return True, "v0.1.0"
    if existing_workflow and existing_workflow.versions:
        new_version = bump_minor_version(
            existing_workflow.versions[0].version)
        should_put = new_version != workflow.version
        return [should_put, new_version]

    return [True, workflow.version]

Also, we're setting should_put on line 43 but implicitly setting it to False at the start of this block. Was that intentional?

[abelanger5]

Ah yeah this looks like a bug and the rewritten code is much better, but still needs to be fixed. I think the logic is correct if auto_version is set to True.

If auto_version isn't set, should_put should be true if the workflow.version does not equal existing_workflow.versions[0].version, or if there is no existing_workflow. So perhaps something like:

def determine_workflow_update(auto_version, workflow, existing_workflow):
    if not auto_version and existing_workflow and existing_workflow.versions:
        return workflow.version != existing_workflow.versions[0].version, workflow.version
    if workflow.version == "":
        return True, "v0.1.0"
    if existing_workflow and existing_workflow.versions:
        new_version = bump_minor_version(
            existing_workflow.versions[0].version)
        should_put = new_version != workflow.version
        return [should_put, new_version]

    return [True, workflow.version]

This seems to match up much better with the Go SDK:

shouldPut := opts.autoVersion

	if err != nil {
		// if not found, create
		if statusErr, ok := status.FromError(err); ok && statusErr.Code() == codes.NotFound {
			shouldPut = true
		} else {
			return fmt.Errorf("could not get workflow: %w", err)
		}

		if workflow.Version == "" && opts.autoVersion {
			req.Opts.Version = "0.1.0"
		}
	} else {
		// if there are no versions, exit
		if len(apiWorkflow.Versions) == 0 {
			return fmt.Errorf("found workflow, but it has no versions")
		}

		// get the workflow version to determine whether to update
		if apiWorkflow.Versions[0].Version != workflow.Version {
			shouldPut = true
		}

		if workflow.Version == "" && opts.autoVersion {
			req.Opts.Version, err = bumpMinorVersion(apiWorkflow.Versions[0].Version)

			if err != nil {
				return fmt.Errorf("could not bump version: %w", err)
			}
		}
	}

[grutt]

Ok, I opened issue #123
Let's close the discussion here and track this there