Skip to content

[CLI] Add --extra flag for deployment describe #3875

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 13, 2025
Merged

[CLI] Add --extra flag for deployment describe #3875

merged 1 commit into from
Oct 13, 2025

Conversation

muhamadazmy
Copy link
Contributor

@muhamadazmy muhamadazmy commented Oct 8, 2025 •
edited
Loading

[CLI] Add --extra flag for deployment describe

Summary:
This is useful on heavy systems to avoid running long and
heavy queries to compute the deployment status and number of
active invocations

Same for deployment list

Fixes #3867

@muhamadazmy muhamadazmy marked this pull request as ready for review October 8, 2025 13:41
@muhamadazmy muhamadazmy requested review from pcholakov and tillrohrmann and removed request for pcholakov October 8, 2025 13:42
@slinkydeveloper slinkydeveloper requested review from slinkydeveloper and removed request for tillrohrmann October 13, 2025 07:26
@slinkydeveloper
Copy link
Contributor

I take over the review of this one.

@slinkydeveloper
Copy link
Contributor

slinkydeveloper commented Oct 13, 2025 •
edited
Loading

Few bits from trying this out:

  • I think we should still have the list of services with their revision in simple deployment list. You get that anyway from the admin API, so no need to hide it.
  • The created at now looks unreadable, perhaps can we have better formatting for that?
 DEPLOYMENT              TYPE  ID                          CREATED-AT                     
 http://localhost:9080/  HTTP  dp_10mWsjoxyxXb0dnwKYatOHn  2025-10-09T13:05:37.557000000Z 
 http://localhost:9080/  HTTP  dp_10jIpvZX2JUxKGGVffXnkm5  2025-10-07T15:33:59.197000000Z 
 http://localhost:9080/  HTTP  dp_12psY87FYUrMsTL75dH1yMh  2025-10-07T14:24:02.553000000Z 
 http://localhost:9080/  HTTP  dp_13ETzZ0hrcwtfeqBENRIK41  2025-10-07T14:19:11.956000000Z

No need for the millisecond part, and make the formatting nicer splitting date and time

  • Can we order the list of deployments by created at?
  • Can we add the sdk_version at least to deployment describe? This is a pretty useful piece of info as well.

slinkydeveloper
slinkydeveloper reviewed Oct 13, 2025
View reviewed changes
pcholakov
pcholakov approved these changes Oct 13, 2025
View reviewed changes
Copy link
Contributor

@pcholakov pcholakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @slinkydeveloper!

I'm sorry @muhamadazmy, I'd been completely ignoring my GH queue for the last few days.

@muhamadazmy
Copy link
Contributor Author

@slinkydeveloper

I think we should still have the list of services with their revision in simple deployment list. You get that anyway from the admin API, so no need to hide it.

In deployment list we never showed the services (by default) but instead the status and active invocation which both require to run a SQL query which is intensive in case we have many invocations. The list of the services is still visible only with the --extra flag

Can we order the list of deployments by created at?

They are! In a descending order

@slinkydeveloper
Copy link
Contributor

The list of the services is still visible only with the --extra flag

I think it should really be in the simple list view as well, it's just a huge crucial piece of information to miss it. Deployments are of no use to a user really without the list of services.

@muhamadazmy
Copy link
Contributor Author

@slinkydeveloper
This is without --extra
image

With --extra
image

slinkydeveloper
slinkydeveloper approved these changes Oct 13, 2025
View reviewed changes
cli/src/ui/datetime.rs Outdated
fn display(&self) -> String {
let dt: DateTime<Local> = self.with_timezone(&Local);

dt.format("%a, %d %h %Y %X %Z").to_string()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personal preference here, but comma is noisy to me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps @nikrooz we should align how we display dates between CLI and UI. Can do in followup PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can remove it, but I was trying to be as close to rfc2822 as possible with minor changes to have a 0 prefixed numbers

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now do as you like, later on we should align CLI/UI on this

@muhamadazmy
[CLI] Add --extra flag for deployment describe
a665265 
Summary:
This is useful on heavy systems to avoid running long and
heavy queries to compute the deployment status and number of
active invocations

Same for `deployment list`

Fixes restatedev#3867
@muhamadazmy muhamadazmy merged commit 9ea36f6 into restatedev:main Oct 13, 2025
50 checks passed
@muhamadazmy muhamadazmy deleted the pr3875 branch October 13, 2025 16:32
@github-actions github-actions bot locked and limited conversation to collaborators Oct 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@slinkydeveloper slinkydeveloper slinkydeveloper approved these changes

@pcholakov pcholakov pcholakov approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[CLI] Simplified view for deployment describe

3 participants

@muhamadazmy @slinkydeveloper @pcholakov