search

argoproj / argo-workflows

Workflow Engine for Kubernetes
https://argo-workflows.readthedocs.io/
Apache License 2.0
15.03k stars 3.2k forks source link

Mentoring request #11464

Closed toblich closed 10 months ago

toblich commented 1 year ago

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 Go and Kubernetes.

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 #11463, which I reported

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: Both sync and async work for me, I'm open to whichever is preferrable for the mentor. My timezone is UTC-3.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is a mentoring request, please provide an update here. Thank you for your contributions.

agilgur5 commented 1 year ago

Hey sorry this hasn't gotten any attention! Workflows gets a good amount of mentoring requests and does not have many people who can do so.

I can help you out with this. For specific questions, let's use the issue & PR. If you have any more general questions, for instance, around the build system etc, let's use this space.

Can message me on CNCF Slack if needed as well, but I'd prefer to keep things in the repo where it has posterity and can help other readers.

toblich commented 1 year ago

Hey @agilgur5, thank you! I've just opened https://github.com/argoproj/argo-workflows/pull/11799, but I have 2 questions first

agilgur5 commented 1 year ago

I've just opened #11799, but I have 2 questions first

Awesome! Thanks for your quick efforts despite the lag time here!

  • Testing: I don't see a trivial way to unit test this case with what's already in place, as the changed feature relies on watching k8s events, so I've opened the PR without any new tests (just manual validation).

Yes this may require an E2E test to confirm. They are in test/e2e/. Though off the top of my head, I'm not sure if there are existing tests for the watch functionality. Sometimes we do rely on manual validation.

In this case, I think it would be more important to confirm that there are no regressions to other functionality, as the watch is already broken for this use-case (so it ain't getting better without changes).

  • The PR template asks for running make pre-commit -B, which fails when I attempt to run it and deletes ~300 files under sdks/java/client/docs (which obviously is completely unrelated to my change). I'm ignoring all these deletions for the time being, but it's unexpected. Do you have any context on why this happens?

If part of the build fails, it can result in the Swagger / OpenAPI codegen being faulty or incomplete. That, in turn, can cause the SDK codegen to then be incorrect. Basically build failure cascade. make clean is usually enough to reset to a clean state and run build from again.

agilgur5 commented 10 months ago

Closing this as #11799 did not get further activity and was later closed by OP in favor of #11855. The formal mentoring program also has been discontinued per #11802 / #11805