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

fix(share/availability): update availability ErrNotAvailable handling logic #2445

Merged
merged 5 commits into from
Jul 26, 2023

Conversation

walldiss
Copy link
Member

@walldiss walldiss commented Jul 7, 2023

Overview

ipldFormat.IsNotFound could no longer reach Availability layer. It will get converted on getter layer to share.ErrNotFound. Update outdated error handling.

@walldiss walldiss added area:shares Shares and samples kind:fix Attached to bug-fixing PRs labels Jul 7, 2023
@walldiss walldiss self-assigned this Jul 7, 2023
@Wondertan
Copy link
Member

Nice catch!

Wondertan
Wondertan previously approved these changes Jul 7, 2023
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

Is there a way to test this? Like we forgot to updated the error check and ideally there would be a test that would catch it

@walldiss walldiss force-pushed the data_not_available_handling branch from 69b4297 to 4cf924f Compare July 7, 2023 17:03
@walldiss
Copy link
Member Author

walldiss commented Jul 7, 2023

I've added test for error conversion. From my experience tests like this gets outdated and deleted very quickly since they highly depended on implementattion

Wondertan
Wondertan previously approved these changes Jul 10, 2023
renaynay
renaynay previously approved these changes Jul 19, 2023
@renaynay renaynay enabled auto-merge (squash) July 19, 2023 12:16
@walldiss walldiss dismissed stale reviews from renaynay and Wondertan via e006ad8 July 19, 2023 12:28
@renaynay renaynay merged commit e354bb5 into celestiaorg:main Jul 26, 2023
18 of 26 checks passed
@MSevey MSevey mentioned this pull request Sep 8, 2023
MSevey added a commit that referenced this pull request Sep 13, 2023
## Overview

**UPDATE**

This PR adds building binaries to the release process by using
[goreleaser](https://goreleaser.com/).

The CI release process is largely unchanged. Releases can be trigger by
either manually triggering the `ci_release` workflow, or they can be
triggered by pushing a version tag.

If the team has not been using the CI to generate releases, so I added a
`make goreleaser-release` command to build the binaries for the release
locally so that they can be manually uploaded.

Result of running `make goreleaser-release`:
```
build
└── goreleaser
    ├── artifacts.json
    ├── celestia-node_Darwin_arm64.tar.gz
    ├── celestia-node_Darwin_x86_64.tar.gz
    ├── celestia-node_Linux_arm64.tar.gz
    ├── celestia-node_Linux_x86_64.tar.gz
    ├── celestia-node_darwin_amd64_v1
    │   └── celestia
    ├── celestia-node_darwin_arm64
    │   └── celestia
    ├── celestia-node_linux_amd64_v1
    │   └── celestia
    ├── celestia-node_linux_arm64
    │   └── celestia
    ├── checksums.txt
    ├── config.yaml
    └── metadata.json
```

The `.goreleaser.yaml` file is 90% stock generated from `goreleaser
init`. The celestia node specific items are:
```yaml
builds:
  - main: ./cmd/celestia
    binary: celestia
    env:
      # NOTE: goreleaser doesn't fully support CGO natively. If CGO is needed
      # for any node features, this should be removed and a workaround might
      # need to be created.
      # REF: https://goreleaser.com/limitations/cgo/
      - CGO_ENABLED=0
      - VersioningPath={{ "github.com/celestiaorg/celestia-node/nodebuilder/node" }}
    goos:
      - linux
      - darwin
    goarch:
      - amd64
      - arm64
    ldflags:
      # Ref: https://goreleaser.com/customization/templates/#common-fields
      #
      # .CommitDate is used to help with reproducible builds, ensuring that the
      # same date is always used
      #
      # .FullCommit is git commit hash goreleaser is using for the release
      #
      # .Version is the version being released
      - -X "{{ .Env.VersioningPath }}.buildTime={{ .CommitDate }}"
      - -X "{{ .Env.VersioningPath }}.lastCommit={{ .FullCommit }}"
      - -X "{{ .Env.VersioningPath }}.semanticVersion={{ .Version }}"
dist: ./build/goreleaser
```

For building locally, i added a `make goreleaser-build` command. The
binaries are put into a `build/goreleaser` directory. The `make
goreleaser` command lists the `goreleaser` commands and also checks the
version as a way to verify you have `goreleaser` installed.

Result of running `make goreleaser-build`:
```         
build
└── goreleaser
    ├── artifacts.json
    ├── celestia-node_darwin_amd64_v1
    │   └── celestia
    ├── config.yaml
    └── metadata.json
```

Successful github action run:
https://github.com/MSevey/celestia-node/actions/runs/6123729144/job/16622244813
Example release generated:
https://github.com/MSevey/celestia-node/releases

Created #2445 as a follow up discussion how to add signing.

---------

Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
Co-authored-by: ramin <raminkeene@gmail.com>
vgonkivs pushed a commit to vgonkivs/celestia-node that referenced this pull request Oct 5, 2023
## Overview

**UPDATE**

This PR adds building binaries to the release process by using
[goreleaser](https://goreleaser.com/).

The CI release process is largely unchanged. Releases can be trigger by
either manually triggering the `ci_release` workflow, or they can be
triggered by pushing a version tag.

If the team has not been using the CI to generate releases, so I added a
`make goreleaser-release` command to build the binaries for the release
locally so that they can be manually uploaded.

Result of running `make goreleaser-release`:
```
build
└── goreleaser
    ├── artifacts.json
    ├── celestia-node_Darwin_arm64.tar.gz
    ├── celestia-node_Darwin_x86_64.tar.gz
    ├── celestia-node_Linux_arm64.tar.gz
    ├── celestia-node_Linux_x86_64.tar.gz
    ├── celestia-node_darwin_amd64_v1
    │   └── celestia
    ├── celestia-node_darwin_arm64
    │   └── celestia
    ├── celestia-node_linux_amd64_v1
    │   └── celestia
    ├── celestia-node_linux_arm64
    │   └── celestia
    ├── checksums.txt
    ├── config.yaml
    └── metadata.json
```

The `.goreleaser.yaml` file is 90% stock generated from `goreleaser
init`. The celestia node specific items are:
```yaml
builds:
  - main: ./cmd/celestia
    binary: celestia
    env:
      # NOTE: goreleaser doesn't fully support CGO natively. If CGO is needed
      # for any node features, this should be removed and a workaround might
      # need to be created.
      # REF: https://goreleaser.com/limitations/cgo/
      - CGO_ENABLED=0
      - VersioningPath={{ "github.com/celestiaorg/celestia-node/nodebuilder/node" }}
    goos:
      - linux
      - darwin
    goarch:
      - amd64
      - arm64
    ldflags:
      # Ref: https://goreleaser.com/customization/templates/#common-fields
      #
      # .CommitDate is used to help with reproducible builds, ensuring that the
      # same date is always used
      #
      # .FullCommit is git commit hash goreleaser is using for the release
      #
      # .Version is the version being released
      - -X "{{ .Env.VersioningPath }}.buildTime={{ .CommitDate }}"
      - -X "{{ .Env.VersioningPath }}.lastCommit={{ .FullCommit }}"
      - -X "{{ .Env.VersioningPath }}.semanticVersion={{ .Version }}"
dist: ./build/goreleaser
```

For building locally, i added a `make goreleaser-build` command. The
binaries are put into a `build/goreleaser` directory. The `make
goreleaser` command lists the `goreleaser` commands and also checks the
version as a way to verify you have `goreleaser` installed.

Result of running `make goreleaser-build`:
```         
build
└── goreleaser
    ├── artifacts.json
    ├── celestia-node_darwin_amd64_v1
    │   └── celestia
    ├── config.yaml
    └── metadata.json
```

Successful github action run:
https://github.com/MSevey/celestia-node/actions/runs/6123729144/job/16622244813
Example release generated:
https://github.com/MSevey/celestia-node/releases

Created celestiaorg#2445 as a follow up discussion how to add signing.

---------

Co-authored-by: Ismail Khoffi <Ismail.Khoffi@gmail.com>
Co-authored-by: ramin <raminkeene@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:shares Shares and samples kind:fix Attached to bug-fixing PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants