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.

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

I want to contribute to issue 11219, I need help from a mentor. #11433

Closed
shmruin opened this issue Jul 24, 2023 · 5 comments
Closed

I want to contribute to issue 11219, I need help from a mentor. #11433

shmruin opened this issue Jul 24, 2023 · 5 comments

Comments

@shmruin
Copy link
Contributor

shmruin commented Jul 24, 2023

Question: Have you read the mentoring guide? Please only submit this request for general mentoring (not GSoC).

Answer: Yes I have read the mentoring guide.

What is your background? Any experience with Go, Kubernetes, React, Typescript, etc.?

Answer: I have previous experience in Java, Kotlin with Spring Boot, and some Go experience

Is there any particular issue you'd like to work on? You may want to check out the list of good first issues.

Answer: I would like to work with issue #11219

Do you want to work with your mentor async (i.e. by Slack messaging or issue comments), or sync (e.g. using Zoom video)? Please include your time-zone if you want to work sync.

Answer: (async). (Timezone - KST)


I already fix the issue but not sure if this change will affect to other codes.
I am lack of knowledge about testing changes.
So I left some mentoring request. Thanks.

@caelan-io
Copy link
Member

Hi @shmruin - welcome to the Argo Workflows community! Feel free to take on #11219 and tag me when you have questions. Thanks!

@shmruin
Copy link
Contributor Author

shmruin commented Jul 25, 2023

Thanks, @caelan-io . I am trying to make a test the #11219 with go test, but it's more and more going messy.
As the original case is using sleep, as below. It seems I have to wait until all workflow is finished.

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: example-steps-simple
  namespace: default
spec:
  entrypoint: main
  templates:
    - name: main
      steps:
        - - name: job-1
            template: sleep
            arguments:
              parameters:
                - name: sleep_duration
                  value: 5
          - name: job-2
            template: sleep
            arguments:
              parameters:
                - name: sleep_duration
                  value: 3

    - name: sleep
      synchronization:
        mutex:
          name: mutex-example-steps-simple
      inputs:
        parameters:
          - name: sleep_duration
      script:
        image: alpine:latest
        command: [/bin/sh]
        source: |
          echo "Sleeping for {{ inputs.parameters.sleep_duration }}"
          sleep {{ inputs.parameters.sleep_duration }}
      memoize:
        key: "memo-key-1"
        cache:
          configMap:
            name: cache-example-steps-simple
         // ...
         cancel, controller := newController(wf)
	defer cancel()

	ctx := context.Background()

	woc := newWorkflowOperationCtx(wf, controller)
	woc.operate(ctx)

	assert.Equal(t, wfv1.WorkflowRunning, woc.wf.Status.Phase)
	job1CacheHit, job2CacheHit := false, false
	for _, node := range woc.wf.Status.Nodes {
		switch node.DisplayName {
		case "job-1":
			fmt.Println(node)
			job1CacheHit = node.MemoizationStatus.Hit
		case "job-2":
			fmt.Println(node)
			job2CacheHit = node.MemoizationStatus.Hit
		default:
			fmt.Println(node.TemplateName)
		}
	}
	assert.False(t, job1CacheHit)
	assert.True(t, job2CacheHit)

So, how does commonly synchronization's tests are configured?
In operator_test.go I cannot find similar to this case. Waiting works to be completed.

@caelan-io
Copy link
Member

Okay, can you link to a draft PR so I have full context?

@shmruin
Copy link
Contributor Author

shmruin commented Jul 27, 2023

@caelan-io, I found the concurrency test file with operator.go and made PR submit with similar test added. Link is here. Very helpful if you could review it. Thanks.

@caelan-io
Copy link
Member

Hey @shmruin - apologies for my delay in replying here. I had my teammate joibel review your PR. Congrats and nice work! Now that you've got your first PR merged, we'll close this issue. Feel free to tag me on any future PRs if questions 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants