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

update payload to generate snapshot #33

Merged
merged 17 commits into from
Oct 9, 2024

Conversation

Aijeyomah
Copy link
Contributor

@Aijeyomah Aijeyomah commented Oct 4, 2024

Notes for Reviewers

This PR fixes #

  • Update payload to invoke github workflow for snapshot
  • Handle error panics
  • temporary remove test which points to mesheryctl. Will add test for knavas-snapshot in another PR

Signed commits

  • Yes, I signed my commits.

@github-actions github-actions bot added the area/ci Continuous integration | Build and release label Oct 4, 2024
@Aijeyomah Aijeyomah force-pushed the update-snapshot-payload branch from 50fa5af to 9f8fd99 Compare October 4, 2024 12:46
@Aijeyomah Aijeyomah marked this pull request as ready for review October 4, 2024 12:46
@Aijeyomah Aijeyomah force-pushed the update-snapshot-payload branch from 5015029 to f9cb5ce Compare October 4, 2024 13:05
@Aijeyomah Aijeyomah requested a review from MUzairS15 October 4, 2024 13:16
Signed-off-by: Eti Ijeoma <[email protected]>
Signed-off-by: Eti Ijeoma <[email protected]>
Signed-off-by: Eti Ijeoma <[email protected]>
Signed-off-by: Eti Ijeoma <[email protected]>
@Aijeyomah Aijeyomah force-pushed the update-snapshot-payload branch from 731f8fb to d7f6ee3 Compare October 7, 2024 13:26
ErrGeneratingSnapshotCode = "kanvas-snapshot-902"
ErrHTTPPostRequestCode = "kanvas-snapshot-903"
ErrDecodingAPICode = "kanvas-snapshot-905"
ErrUnexpectedResponseCodeCode = "kanvas-snapshot-906"
Copy link
Member

Choose a reason for hiding this comment

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

Will you enhance the ErrInvalidChartURI message to be more specific about filesystem and/or URL references?

ErrHTTPPostRequestCode = "kanvas-snapshot-1003"
ErrDecodingAPICode = "kanvas-snapshot-1004"
ErrRequiredFieldNotProvidedCode = "kanvas-snapshot-1005"
ErrInvalidChartURICode = "kanvas-snapshot-900"
Copy link
Member

Choose a reason for hiding this comment

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

ErrCreatingMesheryDesign - offers little insight as to what happened (why the chart wasn't converted to design) and what to do about it or where to go for help.

@@ -1,16 +1,18 @@
package errors

import (
"fmt"

"github.com/layer5io/meshkit/errors"
)

Copy link
Member

Choose a reason for hiding this comment

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

ErrHTTPPostRequest - the message isn't overly accurate or helpful. It would seem that the most likely cause of this failure is

  1. User's network connectivity. Can they reach the currently configured remote provider?
  2. The currently configured remote provider is unavailable / down.
  3. Network connectivity is fine, but their request was malformed. Are they missing a required parameter (an email address) or did they provide an invalid email address?
  4. Network connectivity is fine, but they authentication failed.

.github/.goreleaser.yml Outdated Show resolved Hide resolved
@leecalcote leecalcote merged commit c27fdea into meshery:master Oct 9, 2024
6 of 9 checks passed
@leecalcote
Copy link
Member

@Aijeyomah I was expecting to see some movement on this PR today. I’m going to merge now. I’ll ask that you address the outstanding comments on both this and @MUzairS15's PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci Continuous integration | Build and release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants