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

Build: add more output details to the build detail page #171

Open
agjohnson opened this issue Jun 15, 2023 · 8 comments
Open

Build: add more output details to the build detail page #171

agjohnson opened this issue Jun 15, 2023 · 8 comments
Labels
Accepted Accepted issue on our roadmap Feature New feature Needed: design decision A core team decision is required Status: blocked Issue is blocked on another issue

Comments

@agjohnson
Copy link
Contributor

A secondary discussion that came from readthedocs/readthedocs.org#7005 was around some of the additions lined up for the build detail page, which are roughly tangential to adding a debug flag attribute to the BuildCommand model. These additions would aim to increase the user's understanding of the commands and command steps. There are other additions for the build detail page, but just to focus on improvements to the command listing and/or debug output view:

  • Show the step that the command executed under
  • Show more debug level commands
  • Show command descriptions for some commands

The next step is to experiment more with the display here. All of these changes could negatively affect the build command list, so I'll want to go through a few ideas before settling on what works best here.

A few problems we're solving here:

  • We aren't surfacing where in the build process the command is executed, which would make build.jobs easier to follow and understand
  • We execute commands that aren't clear, for example the cat commands might not make sense if the user isn't familiar with POSIX shells. An annotation or description would help explain what is happening to the user.
@agjohnson agjohnson added Improvement Minor improvement to code Needed: design decision A core team decision is required labels Jun 15, 2023
@agjohnson agjohnson added this to the New dashboard features milestone Jun 15, 2023
@agjohnson
Copy link
Contributor Author

I mocked this up really quick. I shows some options I wanted to play with for showing at least a build step in the command list.

Note this doesn't show descriptions though, which would be another decision to work in here. If we want to show descriptions inline, we'd probably want them to look something like the commented option below. This would mean we need a visually distinct way to show the build step maybe, or we just call everything a comment.

Also note, this is in debug mode only. I think that is the right call for all of this information. It doesn't pollute the view for most users and allows us to be thorough with output for debug commands.

Also also note, the build step could also just be a metadata label similar to the command timing, and therefore displayed on the non-debug output.

Also also also note, we forgot about the raw logs so far too. 😄

Anyways, this is just quick ideas, and I'm not even particularly happy with any, but this will take a few iterations anyways:

image

@humitos
Copy link
Member

humitos commented Jun 15, 2023

re "fantasy commands" from the other issue's conversation: IMO, they are more confusing than helpful. If we think that cat .readthedocs.yaml is not clear enough because people don't know POSIX, I don't think that readthedocs-build --show-config will be clearer. If we want o be clear here, I prefer to show a description instead like Content of the Read the Docs configuration file (docs/user/.readthedocs.yaml) --or similar-- (in gray as shown in your example).

Also, "fantasy commands" may make users think we are executing strange commands on their behalf and think the environment on Read the Docs is different from their local environment and complaining about a build not working on Read the Docs because of these "strange commands". We know those command are not real commands, but they don't.

re using # to show comments in the screen; I'd have the same opinion probably. If we are worried about people not understanding POSIX, I'd avoid using this syntax here as well. So, I'd vote being pretty explicit here to avoid miss understandings and I think the way you are showing by using a different color (gray) is 🎯

@humitos
Copy link
Member

humitos commented Jun 15, 2023

Also also note, the build step could also just be a metadata label similar to the command timing, and therefore displayed on the non-debug output.

I think this could be too noisy for regular use cases.

@agjohnson
Copy link
Contributor Author

Okay, so lets focus on the build steps first, this discussion seems to be stuck on replacing cat.

I think I actually lean towards the second option in my image, using shell like commenting. This option is visually distinct and I actually like how it retains the terminal feel -- both visually, but also in the copy/paste sense. But the front end can display either one with very minor changes.

My questions come down to how to surface the build job/step in our API/modeling/etc. I think if we saved the build step to the BuildCommand, that is enough for the front end. The JS will need some logic around detecting and displaying when the build step changes between commands, but that is fairly minor.

This would allow us to play with the display of this metadata, and the front end is mostly just consuming what the backend says.

I feel it's rather important we display the build step now that we have them. Build jobs and regular builds gain the most here.

@github-project-automation github-project-automation bot moved this to Planned in 📍Roadmap Feb 29, 2024
@agjohnson agjohnson added the Accepted Accepted issue on our roadmap label Feb 29, 2024
@humitos
Copy link
Member

humitos commented Feb 29, 2024

I think if we saved the build step to the BuildCommand, that is enough for the front end

This won't be easy to achieve considering how our current build environment works. We will need to pass it down a step= argument (or similar) in a bunch of places. However, I think we could explore this idea, tho.

I image we could call something like self.build_environment.set_step("create_environment") from the BuildDirector before running any command for that step. Then in BaseBuildEnvironment.record_command() we could read self._environment.step and save it through the API.

After writing that, maybe is not that hard, but at least is not trivial 😅

A different hacker approach could be to execute # Running step system_dependencies using the actual pattern we have and save it as a command it self. This may be simpler to implement, and also simpler to display by the front-end 😄

@agjohnson
Copy link
Contributor Author

Your first plan seems rather doable, though yeah certainly not trivial either.

A different hacker approach could be to execute # Running step system_dependencies

I almost pointed this out as an example of what we should not do 🙃

This would bake these lines into the command output and the front end would not be able to do anything different with these strings later if we wanted to change them. It would also pollute the raw log a bit.

@agjohnson agjohnson mentioned this issue Mar 5, 2024
2 tasks
@agjohnson
Copy link
Contributor Author

#102 is duplicate here to the conversation about adding build steps. We probably want to break this work up into chunks and can address build steps specifically there

@agjohnson agjohnson added the Status: blocked Issue is blocked on another issue label Apr 23, 2024
@humitos humitos added Feature New feature and removed Improvement Minor improvement to code labels Jun 4, 2024
@agjohnson
Copy link
Contributor Author

agjohnson commented Dec 18, 2024

I'm feeling like this feature is overdue, we should discuss how to add this data to the API so we can promote build jobs in the build output.

The only thing I might have shifted on is that I now think we should always show the build step, not just showing the build step in debug mode. I picture this UI as a label with a short string job:install that won't crowd the UI. Maybe debug mode shows the verbose version of this though.

The first batch of work required here is all in the backend:

Lastly is the frontend in this issue:

  • Finally switch to APIv3 for build detail view
  • Show build job field in build detail listing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Feature New feature Needed: design decision A core team decision is required Status: blocked Issue is blocked on another issue
Projects
Status: Planned
Development

No branches or pull requests

2 participants