Show human readable information in queue info

[xoxys]

Fixes: #2948, #5469

Changes to https://github.com/woodpecker-ci/woodpecker/pull/5516/files#diff-fa3f3d2bbef57c87a86886007de51bf6efd76e6820b8cf279dcb398017a3293f were a drive-by fix as I was wondering why the metadata labels are shown as long as a task is pending but disappear after the task started.

[codecov]

Codecov Report

❌ Patch coverage is 5.31915% with 89 lines in your changes missing coverage. Please review.
✅ Project coverage is 26.34%. Comparing base (92ebabe) to head (32a7084).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
server/api/hook.go	0.00%	82 Missing ⚠️
server/pipeline/queue.go	0.00%	6 Missing ⚠️
server/grpc/filter.go	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5516      +/-   ##
==========================================
- Coverage   26.40%   26.34%   -0.07%     
==========================================
  Files         405      405              
  Lines       28942    29028      +86     
==========================================
+ Hits         7643     7646       +3     
- Misses      20600    20683      +83     
  Partials      699      699              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[xoxys]

@anbraten @6543 Can I get another review? 👼

[qwerty287]

I don't have time for a proper review right now, but I was wondering: What happens if you change the name of an agent? Is this reflected on the queue page?

[xoxys]

No with the current way this does not update the name for already running tasks.

[xoxys]

To get the agent name updated immediately, we would need to look up the agent name by ID on every queue info call. There are multiple methods to do it:

1. Using the store as done with 9d406f9#diff-2d7687e7cf4d8db83805f8950671e37eb0af7d3cf0f470b5ae0b78ad93985401
2. Using an http call to GET /agents/{agent_id} If we want to use the http call instead of the store call, which base URL should be used? Making an external call to the public URL is IMO.

[xoxys]

OK I think I found a better way. The agent name lookup is now done on the API level within the GetQueueInfo method. It still uses a store call instead of http call, however I guess on this level its acceptable. This also reflects agent name changes immediatly.

[woodpecker-bot]

Surge PR preview deployment was removed

[qwerty287]

Thanks, could we do that on sql level with join?

[xoxys]

No, at least I dont see a way how this could work (easily).

[qwerty287]

Ah you're right, the queue info is not loaded from db, sorry.
But at least some kind of cache for the agents would be good I think? So we don't query the DB multiple times for the same agent. So with a map[int]*model.Agent

[xoxys]

I had this already but this end up with a lot more code. But I can push it again.

[anbraten]

I had this already but this end up with a lot more code. But I can push it again.

As this introduces such new issues and to keep the api clean, I would still suggest querying the /agents list from the UI for now. We can still add an api property later on, but removing one would be a breaking change again.

[xoxys]

Ok here we go :D added some helper functions to declutter the enrichment while using the cache map approach.

[xoxys]

As this introduces such new issues and to keep the api clean, I would still suggest querying the /agents list from the UI for now. We can still add an api property later on, but removing one would be a breaking change again.

Why? IMO the latest change is a good one and I dont see any benefit to add it to the UI instead.

[qwerty287]

I agree to @xoxys. Querying the agents list should not working as well because of pagination (of course you could work around this by just loading all pages).

[6543]

backend code ack - thanks :)

[anbraten]

Why? IMO the latest change is a good one and I dont see any benefit to add it to the UI instead.

Adding a property as agent_name to the model while having agent_id already is a mix of models IMO as this property adds no new information that was not queryable before and is rather adjusting the model to match our UI views than providing REST models. Feels more like the graphql approach to me. We also need to keep in mind that those properties are send to the agent via grpc (should not be to crazy for a string yet) and api property removals are considered breaking if we change plans. Won't block if the rest is fine with it.

[6543]

well the problem is that either we have models reused or we have type definition for each interface that marks a border to another component of or software to make things more modulare and have cleare seperation of concerns ...

... as there was already the complaint why we have so many "similar" type definitions and the server component is kind of tangled anyway right now i dont care much ... about that concern

about that I'd love to have modules more clear destinct but at the end of the day it still is a single pice of software that we get by cobine our components ...

this here dont save agent_name to database so only if we wana change the json consumers are affected ...

most probably offtopic but the reason why i'm fine with that mix (through if someone works on better cristalizing out components ... i would welcome that)

[qwerty287]

Thanks. Frontend seems good now as well, but I didn't test it (but if you did go on with merging)

[anbraten]

Tasks that have not been picked by an agent point to a not existing pipeline: https://ci.woodpecker-ci.org/repos/3780/pipeline/0/4 (works as soon as an agent picks the task)

The border could be using some darker color in dark-mode:

FYI @xoxys

[xoxys]

Thx for the feedback, will take care 👍