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.

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

FISH-7454 Add timeout option for domain commands #6428

Merged
merged 4 commits into from
Oct 9, 2023
Merged

FISH-7454 Add timeout option for domain commands #6428

merged 4 commits into from
Oct 9, 2023

Conversation

kalinchan
Copy link
Member

@kalinchan kalinchan commented Oct 3, 2023 •
edited
Loading

Description

Commands

  • start-domain
  • restart-domain
  • stop-domain
  • stop-domains
  • stop-all-domains
  • start-domains
  • restart-domains

Testing

Testing Performed

Manual testing

Testing Environment

Ubuntu 22.04.2 LTS, maven 3.8.6 zulu jdk 11

Documentation

payara/Payara-Documentation#325

@kalinchan kalinchan mentioned this pull request Oct 3, 2023
Merged
@Pandrex247
Copy link
Member

Jenkins test please

Pandrex247
Pandrex247 requested changes Oct 4, 2023
View reviewed changes
Copy link
Member

@Pandrex247 Pandrex247 left a comment

Choose a reason for hiding this comment

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

Some comments.

Also, what about:

  • StopDomainsCommand
  • StopAllDomansCommand
  • com.sun.enterprise.v3.admin.StopDomainCommand
  • com.sun.enterprise.v3.admin.RestartDomainCommand
  • StartDomainsCommand
  • RestartDomainsCommand

...in/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StartDomainCommand.java Outdated Show resolved Hide resolved
...min/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StopDomainCommand.java Outdated Show resolved Hide resolved
...min/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StopDomainCommand.java Outdated Show resolved Hide resolved
...admin/server-mgmt/src/main/manpages/com/sun/enterprise/admin/servermgmt/cli/restart-domain.1 Outdated Show resolved Hide resolved
...s/admin/server-mgmt/src/main/manpages/com/sun/enterprise/admin/servermgmt/cli/start-domain.1 Outdated Show resolved Hide resolved
...us/admin/server-mgmt/src/main/manpages/com/sun/enterprise/admin/servermgmt/cli/stop-domain.1 Outdated Show resolved Hide resolved
.../server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/RestartDomainCommand.java Outdated Show resolved Hide resolved
...min/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/cli/StopDomainCommand.java Outdated Show resolved Hide resolved
@kalinchan kalinchan requested a review from Pandrex247 October 6, 2023 09:59
Pandrex247
Pandrex247 requested changes Oct 6, 2023
View reviewed changes
Copy link
Member

@Pandrex247 Pandrex247 left a comment

Choose a reason for hiding this comment

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

I feel like the defaults should be:

  • Start - 600 seconds
  • Stop - 60 seconds
  • Restart - 600 seconds (currently 60)

@kalinchan kalinchan requested a review from Pandrex247 October 6, 2023 11:38
Pandrex247
Pandrex247 requested changes Oct 9, 2023
View reviewed changes
.../admin/server-mgmt/src/main/java/fish/payara/admin/servermgmt/cli/RestartDomainsCommand.java
Copy link
Member

Choose a reason for hiding this comment

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

I think this should command should have the same setup as the start-deployment-group and start-cluster commands where there are two timeout parameters:

  • timeout - overall timeout for the command
  • domainTimeout - timeout for each individual domain

...us/admin/server-mgmt/src/main/java/fish/payara/admin/servermgmt/cli/StartDomainsCommand.java
Copy link
Member

Choose a reason for hiding this comment

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

I think this should command should have the same setup as the start-deployment-group and start-cluster commands where there are two timeout parameters:

  • timeout - overall timeout for the command
  • domainTimeout - timeout for each individual domain

.../admin/server-mgmt/src/main/java/fish/payara/admin/servermgmt/cli/StopAllDomainsCommand.java
Copy link
Member

Choose a reason for hiding this comment

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

I think this should command should have the same setup as the stop-deployment-group and stop-cluster commands where there are two timeout parameters:

  • timeout - overall timeout for the command
  • domainTimeout - timeout for each individual domain

...eus/admin/server-mgmt/src/main/java/fish/payara/admin/servermgmt/cli/StopDomainsCommand.java
Copy link
Member

Choose a reason for hiding this comment

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

I think this should command should have the same setup as the stop-deployment-group and stop-cluster commands where there are two timeout parameters:

  • timeout - overall timeout for the command
  • domainTimeout - timeout for each individual domain

@Pandrex247 Pandrex247 merged commit c6f0272 into payara:master Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pandrex247 Pandrex247 Pandrex247 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kalinchan @Pandrex247