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

MGMT-9451: Moving to new service-agent protocol #76

Open
wants to merge 17 commits into
base: igal/new_agent_protocol
Choose a base branch
from

Conversation

tsorya
Copy link
Owner

@tsorya tsorya commented Mar 22, 2022

Moving new commands : stop and logs
Adding new step type for stop and logs

Assisted Pull Request

Description

List all the issues related to this PR

  • New Feature
  • Bug fix
  • Tests
  • Documentation
  • CI/CD

What environments does this code impact?

  • Automation (CI, tools, etc)
  • Cloud
  • Operator Managed Deployments
  • None

How was this code tested?

  • assisted-test-infra environment
  • dev-scripts environment
  • Reviewer's test appreciated
  • Waiting for CI to do a full test run
  • Manual (Elaborate on how it was tested)
  • No tests needed

Assignees

/cc @
/cc @

Checklist

  • Title and description added to both, commit and PR.
  • Relevant issues have been associated (see CONTRIBUTING guide)
  • Reviewers have been listed
  • This change does not require a documentation update (docstring, docs, README, etc)
  • Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

  • Are the title and description (in both PR and commit) meaningful and clear?
  • Is there a bug required (and linked) for this change?
  • Should this PR be backported?

Moving new commands : stop and logs
Adding new step type for stop and logs
// LogsGatherCmdRequest logs gather cmd request
//
// swagger:model logs_gather_cmd_request
type LogsGatherCmdRequest struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is great, but how are you planning to deliver it? it seems that in order to do unmarshel in the agent side the agent will need to import this version of the service, I'm unsure you can deliver this change independently.

package models

// This file was generated by the swagger tool.
// Editing this file might prove futile when you re-run the swagger generate command
Copy link
Collaborator

Choose a reason for hiding this comment

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

Try rerunning without changing the swagger and ensure the validations aren't changed.

return nil
}

func (m *LogsGatherCmdRequest) validateInsecure(formats strfmt.Registry) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need formats here?

res = append(res, err)
}

if len(res) > 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If all validations pass res will be [nil, nil, nil, nil...], no ?


func (m *LogsGatherCmdRequest) validateInsecure(formats strfmt.Registry) error {

if err := validate.Required("insecure", "body", m.Insecure); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What are you validating here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants