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

infra: Add debugging tool in action(win build) #1688

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JSUYA
Copy link
Member

@JSUYA JSUYA commented Oct 6, 2023

tmate is a tool that helps debugging github actions.
If CI fails, opens an ssh connection for debugging.
It is difficult to predict the msvc dev environment of the window-latest image.
This can be useful when problems occurs.

https://github.com/marketplace/actions/debugging-with-tmate

tmate is a tool that helps debugging github actions.
If ci fails, opens an ssh connection for debugging.
It is difficult to predict the msvc dev environment of the window-latest image.
This can be useful when problems occurs.

https://github.com/marketplace/actions/debugging-with-tmate
@JSUYA JSUYA requested a review from hermet as a code owner October 6, 2023 04:46
@hermet hermet added the infrastructure Dev infrastructure label Oct 6, 2023
@hermet
Copy link
Member

hermet commented Oct 9, 2023

thanks. feedbacks:

  1. If you find this useful, it might be better to apply it for all supported platforms. Some engineers use Windows for development, while Linux/macOS is controversial.

  2. This usage is optional, and tmate will be terminated after a designated time. So, I believe applying this guide would be the best option for us. https://github.com/marketplace/actions/debugging-with-tmate#manually-triggered-debug

@JSUYA
Copy link
Member Author

JSUYA commented Oct 17, 2023

  1. This usage is optional, and tmate will be terminated after a designated time. So, I believe applying this guide would be the best option for us. https://github.com/marketplace/actions/debugging-with-tmate#manually-triggered-debug

I researched about using workflow_dispatch.
In my opinion, this is not suitable for our goal. (for testing, debugging)
The reason we set up ssh debugging(tmate) is because we want to debug the CI's environment status during the Pull-Request step.
workflow_dispatch is not a tool that can be set in the PR step. (like this)
This is used when operating for a separate event in the Action tab.
Therefore, I think it is better to use when build failed and use the timeout appropriately.

@hermet
Copy link
Member

hermet commented Oct 17, 2023

@JSUYA The benefit of the manual trigger method is that we can trigger Tmate only when the test fails for a valid reason. Otherwise, every time you push commits, Tmate will add to our test time, and you'll have to wait for the GitAction result to use Tmate. Unfortunately, many many of time we don't have fails. This can be quite burdensome. The key point to note is that Tmate will be terminated in 15 minutes, I think that's why the Tmate guides the manual mode.

@JSUYA
Copy link
Member Author

JSUYA commented Oct 17, 2023

@JSUYA The benefit of the manual trigger method is that we can trigger Tmate only when the test fails for a valid reason. Otherwise, every time you push commits, Tmate will add to our test time, and you'll have to wait for the GitAction result to use Tmate. Unfortunately, many many of time we don't have fails. This can be quite burdensome. The key point to note is that Tmate will be terminated in 15 minutes, I think that's why the Tmate guides the manual mode.

@hermet
workflow_dispatch(in tmate's manual trigger's guide) guide can be executed targeting the default branch.
The trigger does not work because the build check target is the dev branch or the dev branch of a personal repository.
Unless I misunderstand.. The build status of our default branch almost always has no problems, so it does not fit the current purpose. https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_dispatch

Our build and unit_test build tests are each built in the same environment.
Since the build environment(state?) of unit_test includes the build environment of build,
if the build fails, the failure is displayed starting from unit_test.

In fact, we can only put this code(commit) in and test it when a problem occurs.
And I think, there might be a more efficient way.
But I haven't found a way yet and other projects I know are already using this, so I don't think we need to worry about this further.
We don't necessarily need this feature.

@hermet
Copy link
Member

hermet commented Oct 19, 2023

workflow_dispatch(tmate) works on a branch:
image

Now, I think we need to consider these points:

a. We haven't received any requests for this so far. Does this really work for debugging? Can it help us figure out the problem in 15 minutes?
b. What about Ubuntu, macOS, and other platforms? Currently, we officially manage the following platforms: Ubuntu, Windows, macOS, iOS, and Android.

@JSUYA
Copy link
Member Author

JSUYA commented Oct 19, 2023

workflow_dispatch(tmate) works on a branch:

Oh I didn't see there was a dropbox to select branch there. Thank you for check!

a. We haven't received any requests for this so far. Does this really work for debugging? Can it help us figure out the problem in 15 minutes? b. What about Ubuntu, macOS, and other platforms? Currently, we officially manage the following platforms: Ubuntu, Windows, macOS, iOS, and Android.

a.There was no any request. I added this just because I thought it might be useful. While talking with a team member about Windows CI, he recommended tmate. I also was worried about the 15 minutes but figured this could be solved. However, we don't have to worry about it because i will change it to trigger code.
b. I will add it to other platform build scripts as well.

@JSUYA JSUYA closed this Nov 3, 2023
@JSUYA JSUYA reopened this Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Dev infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants