-
Notifications
You must be signed in to change notification settings - Fork 973
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
Elastic agent status report endpoint #4164
Elastic agent status report endpoint #4164
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -32,16 +32,27 @@ def show | |||
render_plugin_error e | |||
end | |||
|
|||
def show | |||
@view_title = 'Plugin Status Report' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we change it to Agent status report(agentid)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, should the actions be agent_status
and plugin_status
over index
and show
?
@@ -29,7 +29,11 @@ | |||
<span>Not yet assigned</span> | |||
{/if} | |||
#if ( $elasticProfilePluginId && $userHasAdministratorRights ) | |||
<a href="$req.getContextPath()/admin/status_reports/$elasticProfilePluginId" class="btn-primary btn-small status-report-btn-small">Check Plugin Status</a> | |||
{if build.build_assigned_date != -1} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gocd/committers --- With this implementation if plugin has not implemented agent-status-report
call it will raise UnhandledRequestTypeException
. We will update the document saying both the call is mandatory if plugin supports status_report capability. Is this okay ?
Seems reasonable. Unless you are looking to add multiple capabilities for
each report page.
…On Mon, Jan 8, 2018, 6:26 PM Bhupendra ***@***.***> wrote:
***@***.**** approved this pull request.
LGTM
------------------------------
In
server/webapp/WEB-INF/rails.new/app/controllers/admin/status_reports_controller.rb
<#4164 (comment)>:
> @@ -32,16 +32,27 @@ def show
render_plugin_error e
end
+ def show
+ @view_title = 'Plugin Status Report'
Can we change it to Agent status report(agentid) ?
------------------------------
In
server/webapp/WEB-INF/vm/build_detail/_build_detail_summary_jstemplate.vm
<#4164 (comment)>:
> @@ -29,7 +29,11 @@
<span>Not yet assigned</span>
{/if}
#if ( $elasticProfilePluginId && $userHasAdministratorRights )
- <a href="$req.getContextPath()/admin/status_reports/$elasticProfilePluginId" class="btn-primary btn-small status-report-btn-small">Check Plugin Status</a>
+ {if build.build_assigned_date != -1}
@gocd/committers <https://github.com/orgs/gocd/teams/committers> --- With
this implementation if plugin has not implemented agent-status-report
call it will raise UnhandledRequestTypeException. We will update the
document saying both the call is mandatory if plugin supports
status_report
<https://github.com/gocd/gocd/blob/master/plugin-infra/go-plugin-domain/src/main/java/com/thoughtworks/go/plugin/domain/elastic/Capabilities.java#L26>
capability. Is this okay ?
—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
<#4164 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAApZmAcvgVQwyJTire6SdJBNQZ1_k2Bks5tIhBvgaJpZM4RWMbs>
.
|
def show | ||
@view_title = 'Plugin Status Report' | ||
@page_header = 'Plugin Status Report' | ||
@agent_status_report = elastic_agent_extension.getAgentStatusReport(params[:plugin_id], params[:elastic_agent_id]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should be the behaviour when an agent is not assigned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please correct me if I am wrong. I thought an agent status report would be more useful in cases where an agent is not yet assigned. I thought the plugin through the agent status report should be able to answer questions around the status of the agent creation, wouldn't this be possible since we send the job identifier to the plugin.
Changes done as part of the current PR:
Note: Based on the information available in the context from which the |
cc3ad6a
to
3be6fbe
Compare
* In case of job being assigned to an agent, show 'Check Agent Status' button which will redirect to elastic agent status report page * In case of job not being assigned yet, show 'Check Plugin Status' button which will redirect to plugin status report page
…nt time instead of agent uuid string check
* Show status report link only for elastic agents * Show original agent job history page link in case of plugin doesn't support status reports
* Rename method from index and show to plugin_status and agent_status respectively * Fix view title and page header * Change the agent-status-report page to include job_id as route instead of elastic_agent_id
* Agent status report request receives elastic_agent_id and job_identifier as part of request body * If the job is not yet assigned, the elastic_agent_id field will be null * The route url for agent status report is '/go/admin/status_reports/<ELASTIC_PLUGIN_ID>/<JOB_ID>'
3be6fbe
to
478dde1
Compare
TO BE MERGED for 18.2
Done
Pending