Skip to content

Conversation

@dcelasun
Copy link
Member

This fixes https://github.com/cloudquery/cloudquery-issues/issues/616

Summary


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines 🧑‍🎓
  • Run go fmt ./... to format your code 🖊
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests 🧪
  • Ensure the status checks below are successful ✅

Copy link
Member

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

Nice 👍 I know this is still in draft, but added a couple of non blocking comments regarding error handling. Please let me know what you think

@dcelasun dcelasun marked this pull request as ready for review October 23, 2024 10:51
@dcelasun dcelasun requested a review from a team October 23, 2024 10:51
@marianogappa
Copy link
Contributor

@dcelasun have you been able to test this with the binary? I'm double-checking here because I'm not seeing the error:

Before patching with your latest commit

✓ Code/test-aws-sync  $ cli sync .                                                              ⏱ 14:48:13
Loading spec(s) from .
Error: failed to get source plugin (name: cloudquery/aws@v270.23.0) information: 404 Not Found

After

💩 Code/test-aws-sync  $ cli sync .                                                             ⏱ 14:48:31
Loading spec(s) from .
Error: failed to get source plugin (name: cloudquery/aws@v270.23.0) information: 404 Not Found

Double-checking here still in case I did something wrong while testing

@dcelasun
Copy link
Member Author

dcelasun commented Oct 23, 2024

@dcelasun have you been able to test this with the binary? I'm double-checking here because I'm not seeing the error:

Fixed by the latest commit, though I'm not sure if that's the cleanest approach. One idea is to pull the version check to its own function, before isDockerPlugin and DownloadPluginFromHub but that would mean repeating network requests on the happy path.

Open to suggestions.

image

@dcelasun dcelasun added the automerge Add to automerge PRs once requirements are met label Oct 23, 2024
@kodiakhq kodiakhq bot merged commit b4ca098 into main Oct 23, 2024
6 checks passed
@kodiakhq kodiakhq bot deleted the feat/better_404_handling branch October 23, 2024 12:46
kodiakhq bot pushed a commit that referenced this pull request Oct 23, 2024
🤖 I have created a release *beep* *boop*
---


## [1.25.0](v1.24.1...v1.25.0) (2024-10-23)


### Features

* Link to latest version on 404 hub downloads ([#423](#423)) ([b4ca098](b4ca098))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Add to automerge PRs once requirements are met feat

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants