Remove hard-coded record types #132

nabarun · 2024-01-11T11:05:01Z

nabarun commented

2024-01-11 11:05:01 +00:00

Resolves #107 and is initial work on #108
- Refactors the Record.Attributes from Any into a byte string and removes the hard-coded Protobuf record types.
- Fixes EIP-712 bytes decoding.
Resolves #109
- Rewords the graphql schema to be able to represent generic IPLD objects encoded as DAG-JSON.

- Resolves #107 and is initial work on #108 - Refactors the `Record.Attributes` from Any into a byte string and removes the hard-coded Protobuf record types. - Fixes EIP-712 bytes decoding. - Resolves #109 - Rewords the graphql schema to be able to represent generic IPLD objects encoded as DAG-JSON.

nabarun added 17 commits 2024-01-11 11:05:01 +00:00

remove hard-coded record types, fix EIP-712 decoding 6dbb27f46c

rename types for clarity

type rename a9c04a7980

this matches the change in follow up PR branch `graphql-to-ipld`

rm confusing comments cffaf547a9

Fix lint errors f51de891e9

normalize logger e6c27c68f1

gql: move schema to cerc-io/ 505b8fb0b7

gql schema for ipld objects 378c6d7cde

gql -> rpc dag-json encoding 62d2d1598a

use sum type in protobuf query 8d7e26515b

encoding of query attrs 69cf8f7b56

clean up 9bf5fa589c

clean up df6d8d8234

type rename 4f8fda7c9c

Resolve rebase conflicts 6f6138f02a

Skip GRPC query test with recursive attributes input e004584685

Fix golang-ci lint errors c3e0102571

Fix protobuf lint error

Build / cleanup-runs (pull_request) Has been cancelled

Details

Build / build (pull_request) Has been cancelled

Details

Lint / Run golangci-lint (pull_request) Has been cancelled

Details

Lint / Run flake8 on python integration tests (pull_request) Has been cancelled

Details

Protobuf / lint (pull_request) Has been cancelled

Details

Protobuf / break-check (pull_request) Has been cancelled

Details

CodeQL / Analyze (go) (pull_request) Has been cancelled

Details

Dependency Review / dependency-review (pull_request) Has been cancelled

Details

Deploy Contract / cleanup-runs (pull_request) Has been cancelled

Details

Deploy Contract / deploy (pull_request) Has been cancelled

Details

Pull Request Labeler / triage (pull_request) Has been cancelled

Details

Run Gosec / Gosec (pull_request) Has been cancelled

Details

Semgrep / Scan (pull_request) Has been cancelled

Details

Tests / cleanup-runs (pull_request) Has been cancelled

Details

Tests / test-unit-cover (pull_request) Has been cancelled

Details

Tests / test-importer (pull_request) Has been cancelled

Details

Tests / test-rpc (pull_request) Has been cancelled

Details

Tests / sdk_tests (pull_request) Has been cancelled

Details

10210aea31

nabarun referenced this pull request from cerc-io/laconic-sdk

2024-01-11 11:10:00 +00:00

Remove hard-coded record types #52

dboreham requested review from telackey 2024-01-11 14:02:55 +00:00

telackey commented

2024-01-11 19:50:46 +00:00

I built and deployed laconicd, built the new laconic-sdk, and then built the CLI to use the new SDK.

After all that, I am getting some errors:

> laconic -c ~/.laconic/config.yml cns record list

[
  {
    message: 'strconv.ParseFloat: parsing "-": invalid syntax',
    path: [ 'queryRecords' ]
  }
]

❯ laconic -c ~/.laconic/local.yml cns record get --id bafyreibctvtieli3gqs7zkaxmwt4wbtakcrurjgrvd44c3mepr3h2qo2ju

[
  {
    message: 'Invalid byte while expecting start of value: 0x3f',
    path: [ 'getRecordsByIds' ]
  }
]

# Note: should be several records

❯ laconic -c ~/.laconic/local.yml cns record list --all --type ApplicationRecord
[]

What might I be doing wrong?

I built and deployed laconicd, built the new laconic-sdk, and then built the CLI to use the new SDK. After all that, I am getting some errors: ``` > laconic -c ~/.laconic/config.yml cns record list [ { message: 'strconv.ParseFloat: parsing "-": invalid syntax', path: [ 'queryRecords' ] } ] ``` ``` ❯ laconic -c ~/.laconic/local.yml cns record get --id bafyreibctvtieli3gqs7zkaxmwt4wbtakcrurjgrvd44c3mepr3h2qo2ju [ { message: 'Invalid byte while expecting start of value: 0x3f', path: [ 'getRecordsByIds' ] } ] ``` ``` # Note: should be several records ❯ laconic -c ~/.laconic/local.yml cns record list --all --type ApplicationRecord [] ``` What might I be doing wrong?

telackey commented

2024-01-11 20:27:17 +00:00

I deleted the existing registry data and recreated it from scratch, and I was able to get working responses. So apparently this is a completely breaking change for existing deployments. That's probably OK, just good to note.

I did see something quirky though. I am not sure if the is a CLI or server issue, but laconic cns record list now shows all records even when --all is not specified if no other filters are included.

If any filter is included, it reverts to the old behavior and only shows results with names, unless --all is set explicitly. This is rather confusing, but also comparatively minor.

I deleted the existing registry data and recreated it from scratch, and I was able to get working responses. So apparently this is a completely breaking change for existing deployments. That's probably OK, just good to note. I did see something quirky though. I am not sure if the is a CLI or server issue, but `laconic cns record list` now shows all records even when `--all` is not specified if no other filters are included. If any filter _is_ included, it reverts to the old behavior and only shows results with names, unless `--all` is set explicitly. This is rather confusing, but also comparatively minor.

telackey approved these changes 2024-01-11 22:53:00 +00:00

telackey left a comment

Other than the "all" issue mentioned above, LGTM, and I am fine if we just open an issue for that to be fixed later.

telackey commented

2024-01-13 01:58:55 +00:00

Since this is a breaking change, we probably want to merge it and the SDK PR and create new releases sooner rather than later, so that I can re-create the production registry deployment before it contains any valuable data.

ashwin merged commit 776799ea02 into main

2024-01-15 04:20:39 +00:00

ashwin deleted branch ng-rm-record-types

2024-01-15 04:20:40 +00:00

ashwin referenced this issue from a commit

2024-01-15 04:20:41 +00:00

Remove hard-coded record types (#132)

ashwin referenced this pull request

2024-01-15 04:21:40 +00:00

Remove hard-coded record types #110

ashwin referenced this pull request

2024-01-15 04:22:10 +00:00

Graphql schema for DAG-JSON objects #112

ashwin referenced this issue from a commit

2024-01-15 04:58:57 +00:00

Remove hard-coded record types (#52)

prathamesh commented

2024-01-15 11:21:29 +00:00

I did see something quirky though. I am not sure if the is a CLI or server issue, but laconic cns record list now shows all records even when --all is not specified if no other filters are included.

On checking locally, I see that command laconic cns record list without the --all flag returns all records even with previous laconicd and laconic-sdk versions (laconicd - 9c240f1a0c152d74a4bea57bfd6af7f323d44b5c, laconic-registry-cli - 6814707752c445ae9cfec9fd282a4f88adac08f8)

> I did see something quirky though. I am not sure if the is a CLI or server issue, but `laconic cns record list` now shows all records even when `--all` is not specified if no other filters are included. > On checking locally, I see that command `laconic cns record list` without the `--all` flag returns all records even with previous laconicd and laconic-sdk versions (laconicd - `9c240f1a0c152d74a4bea57bfd6af7f323d44b5c`, laconic-registry-cli - `6814707752c445ae9cfec9fd282a4f88adac08f8`)

Sign in to join this conversation.

No reviewers