TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 105 Pull requests 13 Projects Releases 2 Activity Actions

Add logs-linter #526

Merged
fyrchik merged 3 commits from achuprov/frostfs-node:logs-linter into master 2024-09-04 19:51:01 +00:00
Conversation 10 Commits 3 Files changed 11 +67 -23
achuprov commented 2023-07-17 10:15:11 +00:00
Member
Copy link

Close #162

Adding logs-linter

Not a elegant solution
Perhaps it should be taken out into a separate action.yml in the linter repository
7b53f8ff3a/.forgejo/workflows/tests.yml (L19-L21)
But this requires additional study, whether it is possible to connect action.yml files not from github. I couldn't achieve this locally using act, however, it's possible that forgejo might allow for it.

Close #162 Adding logs-linter Not a elegant solution Perhaps it should be taken out into a separate action.yml in the linter repository https://git.frostfs.info/achuprov/frostfs-node/src/commit/7b53f8ff3ac29d4b5583d649c56d721f7d3f2103/.forgejo/workflows/tests.yml#L19-L21 But this requires additional study, whether it is possible to connect action.yml files not from github. I couldn't achieve this locally using act, however, it's possible that forgejo might allow for it.
dstepanov-yadro changed title from [#162] add logs-linter to WIP: [#162] add logs-linter 2023-07-17 12:35:36 +00:00
achuprov force-pushed logs-linter from 17eb079cdf to b99a18bcc8 2023-07-25 10:30:24 +00:00 Compare
achuprov force-pushed logs-linter from b99a18bcc8 to 48c3dcb989 2023-07-25 14:13:01 +00:00 Compare
achuprov force-pushed logs-linter from 48c3dcb989 to 1476590cce 2023-07-25 14:28:51 +00:00 Compare
achuprov force-pushed logs-linter from 1476590cce to 3885ca0e8e 2023-07-25 14:59:07 +00:00 Compare
achuprov changed title from WIP: [#162] add logs-linter to [#162] add logs-linter 2023-07-25 15:32:54 +00:00
requested reviews from storage-core-committers, storage-core-developers 2023-07-26 07:26:07 +00:00
aarifullin approved these changes 2023-07-26 10:00:41 +00:00
dstepanov-yadro requested changes 2023-07-26 10:34:25 +00:00
.golangci.yml Outdated
@ -40,0 +41,4 @@
noliteral:
path: .bin/external_linters.so
description:
original-url: git.frostfs.info/achuprov/linter
dstepanov-yadro commented 2023-07-26 10:33:32 +00:00
Member
Copy link

Invalid URL

Invalid URL
achuprov commented 2023-07-26 12:15:53 +00:00
Author
Member
Copy link

fixed

fixed
dstepanov-yadro marked this conversation as resolved
Makefile Outdated
@ -133,8 +133,12 @@ pre-commit-run:
# Run linters
lint:
@git clone https://git.frostfs.info/achuprov/linter.git
dstepanov-yadro commented 2023-07-26 10:34:00 +00:00
Member
Copy link

Invalid URL

Invalid URL
achuprov commented 2023-07-26 12:15:43 +00:00
Author
Member
Copy link

fixed

fixed
dstepanov-yadro marked this conversation as resolved
achuprov force-pushed logs-linter from 3885ca0e8e to 956821812f 2023-07-26 11:50:57 +00:00 Compare
dstepanov-yadro approved these changes 2023-07-26 12:57:55 +00:00
fyrchik requested changes 2023-07-26 15:08:20 +00:00
.forgejo/workflows/tests.yml Outdated
@ -16,1 +16,4 @@
- name: Install linters
run: |
git clone --depth 1 https://git.frostfs.info/TrueCloudLab/linters.git
fyrchik commented 2023-07-26 15:08:18 +00:00
Owner
Copy link

See staticcheck example below, we would not like to duplicate install instructions.
Maybe introduce make extralinters-install? Or even make lint-install, it depends.

See `staticcheck` example below, we would not like to duplicate install instructions. Maybe introduce `make extralinters-install`? Or even `make lint-install`, it depends.
achuprov commented 2023-07-27 15:48:12 +00:00
Author
Member
Copy link

done

done
fyrchik marked this conversation as resolved
.golangci.yml Outdated
@ -39,1 +39,4 @@
alias: objectSDK
custom:
noliteral:
path: .bin/external_linters.so
fyrchik commented 2023-07-26 15:04:54 +00:00
Owner
Copy link

Will my local make lint continue to work if I don't have this?
Can we restrict it to actions somehow?

Will my local `make lint` continue to work if I don't have this? Can we restrict it to actions somehow?
fyrchik commented 2023-07-28 10:42:09 +00:00
Owner
Copy link

I am not sure .bin is the right name, how about just bin (we already store protogen plugin there) or .cache (it is in the git ignore).

I am not sure `.bin` is the right name, how about just `bin` (we already store `protogen` plugin there) or `.cache` (it is in the git ignore).
👍 1
Makefile Outdated
@ -133,8 +133,12 @@ pre-commit-run:
# Run linters
lint:
@git clone --depth 1 https://git.frostfs.info/TrueCloudLab/linters.git
fyrchik commented 2023-07-26 15:06:01 +00:00
Owner
Copy link

We will clone a git repo on each make lint.
This is not ideal, we should be able to do make lint even without the network.

We will clone a git repo on _each_ `make lint`. This is not ideal, we should be able to do `make lint` even without the network.
achuprov commented 2023-07-27 15:51:33 +00:00
Author
Member
Copy link

make lint command no longer needs internet connectivity

`make lint` command no longer needs internet connectivity
fyrchik marked this conversation as resolved
achuprov force-pushed logs-linter from 4e33e93e35 to 8aed013a84 2023-07-27 14:42:13 +00:00 Compare
achuprov force-pushed logs-linter from 8aed013a84 to 0429b26743 2023-07-27 14:55:19 +00:00 Compare
achuprov force-pushed logs-linter from 0429b26743 to 0a0d722a56 2023-07-27 15:40:44 +00:00 Compare
achuprov force-pushed logs-linter from 0a0d722a56 to a4c0cc471b 2023-07-27 15:42:12 +00:00 Compare
achuprov force-pushed logs-linter from a4c0cc471b to fb04ab8e76 2023-07-27 15:43:48 +00:00 Compare
dstepanov-yadro approved these changes 2023-07-27 15:46:02 +00:00
Makefile Outdated
@ -134,0 +137,4 @@
@git clone --depth 1 https://git.frostfs.info/TrueCloudLab/linters.git
@cd linters && make lib OUT_DIR=../.bin/
@cd ..
@export GOBIN=`pwd`/.bin && go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.53.3
dstepanov-yadro commented 2023-07-27 15:06:19 +00:00
Member
Copy link

I think it's better to not export, just GOBIN=/path/bin go install ...

I think it's better to not export, just `GOBIN=/path/bin go install ...`
dstepanov-yadro marked this conversation as resolved
fyrchik reviewed 2023-07-28 10:43:20 +00:00
.forgejo/workflows/tests.yml Outdated
@ -19,2 +17,2 @@
with:
version: latest
- name: Install linters
run: |
fyrchik commented 2023-07-28 10:40:18 +00:00
Owner
Copy link

Can we use single line syntax for a single command run: make lint-install -- like in other places in this file.

Can we use single line syntax for a single command `run: make lint-install` -- like in other places in this file.
Makefile Outdated
@ -133,1 +134,4 @@
# Install linters
lint-install:
@git clone --depth 1 https://git.frostfs.info/TrueCloudLab/linters.git
fyrchik commented 2023-07-28 10:43:05 +00:00
Owner
Copy link

It is a bad idea to clone something in the repo root -- the comman can fail in the middle and we will have garbage. How about .cache or tmp?

It is a bad idea to clone something in the repo root -- the comman can fail in the middle and we will have garbage. How about `.cache` or `tmp`?
achuprov force-pushed logs-linter from fb04ab8e76 to fdf36754e6 2023-07-28 12:54:34 +00:00 Compare
achuprov force-pushed logs-linter from fdf36754e6 to 196e420fa4 2023-07-28 15:43:10 +00:00 Compare
acid-ant changed title from [#162] add logs-linter to Add logs-linter 2023-07-31 11:59:16 +00:00
acid-ant requested changes 2023-07-31 12:27:39 +00:00
Makefile Outdated
@ -25,6 +25,7 @@ PKG_VERSION ?= $(shell echo $(VERSION) | sed "s/^v//" | \
sed -E "s/(.*)-(g[a-fA-F0-9]{6,8})(.*)/\1\3~\2/" | \
sed "s/-/~/")-${OS_RELEASE}
acid-ant commented 2023-07-31 12:13:04 +00:00
Member
Copy link

Why we need this new line?

Why we need this new line?
acid-ant marked this conversation as resolved
scripts/export-metrics/main.go Outdated
@ -1,52 +0,0 @@
package main
acid-ant commented 2023-07-31 12:07:35 +00:00
Member
Copy link

Why we need to remove this file? How this changes relates to your commit message?

Why we need to remove this file? How this changes relates to your commit message?
achuprov commented 2023-08-02 07:42:46 +00:00
Author
Member
Copy link

That was an unsuccessful Friday commit. Rolled it back

That was an unsuccessful Friday commit. Rolled it back
acid-ant marked this conversation as resolved
achuprov changed title from Add logs-linter to WIP: Add logs-linter 2023-07-31 12:40:06 +00:00
fyrchik reviewed 2023-07-31 14:54:03 +00:00
Makefile Outdated
@ -134,3 +141,2 @@
# Run linters
lint:
@golangci-lint --timeout=5m run
lint-run:
fyrchik commented 2023-07-31 14:54:03 +00:00
Owner
Copy link

This is a common target across multiple our repos, I would leave it as is.

This is a common target across multiple our repos, I would leave it as is.
achuprov commented 2023-08-02 07:47:21 +00:00
Author
Member
Copy link

reverted

reverted
fyrchik marked this conversation as resolved
achuprov force-pushed logs-linter from 196e420fa4 to 62e17c8542 2023-08-01 13:35:09 +00:00 Compare
achuprov force-pushed logs-linter from 62e17c8542 to e47059ea2e 2023-08-01 13:39:45 +00:00 Compare
achuprov changed title from WIP: Add logs-linter to Add logs-linter 2023-08-01 13:48:24 +00:00
requested reviews from acid-ant, fyrchik 2023-08-02 08:35:09 +00:00
dstepanov-yadro requested changes 2023-08-02 15:01:42 +00:00
Makefile Outdated
@ -134,0 +134,4 @@
# Install linters
lint-install:
@mkdir -p tmp && cd tmp && git clone --depth 1 https://git.frostfs.info/TrueCloudLab/linters.git && cd linters && make lib OUT_DIR=../../.bin/
@GOBIN=$(shell pwd)/.bin go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.53.3
dstepanov-yadro commented 2023-08-02 15:01:39 +00:00
Member
Copy link

Why again .bin ? You can use $(BIN)

Why again `.bin` ? You can use $(BIN)
achuprov force-pushed logs-linter from e47059ea2e to 1f0bb74f95 2023-08-02 18:00:19 +00:00 Compare
achuprov force-pushed logs-linter from 1f0bb74f95 to fbf106fb65 2023-08-02 18:58:43 +00:00 Compare
achuprov commented 2023-08-04 11:52:20 +00:00
Author
Member
Copy link

Table of const check linter triggers

click
File Line:Column Error Description
assemble.go 18:3 noliteral Wrong package for constants
get.go 110:4 noliteral Wrong package for constants
calls.go 19:15 noliteral Wrong package for constants
calls.go 30:15 noliteral Wrong package for constants
calls.go 41:15 noliteral Wrong package for constants
calls.go 57:15 noliteral Wrong package for constants
calls.go 68:15 noliteral Wrong package for constants
calls.go 86:15 noliteral Wrong package for constants
calls.go 89:15 noliteral Wrong package for constants
calls.go 97:15 noliteral Wrong package for constants
convert.go 19:16 noliteral Wrong package for constants
doctor.go 15:15 noliteral Wrong package for constants
doctor.go 19:15 noliteral Wrong package for constants
doctor.go 27:15 noliteral Wrong package for constants
doctor.go 34:15 noliteral Wrong package for constants
evacuate.go 25:15 noliteral Wrong package for constants
evacuate.go 35:15 noliteral Wrong package for constants
evacuate.go 46:15 noliteral Wrong package for constants
evacuate_async.go 17:15 noliteral Wrong package for constants
evacuate_async.go 30:16 noliteral Wrong package for constants
evacuate_async.go 32:15 noliteral Wrong package for constants
evacuate_async.go 41:15 noliteral Wrong package for constants
evacuate_async.go 49:15 noliteral Wrong package for constants
evacuate_async.go 56:16 noliteral Wrong package for constants
evacuate_async.go 58:15 noliteral Wrong package for constants
evacuate_async.go 68:15 noliteral Wrong package for constants
evacuate_async.go 76:15 noliteral Wrong package for constants
evacuate_async.go 83:16 noliteral Wrong package for constants
evacuate_async.go 85:15 noliteral Wrong package for constants
evacuate_async.go 94:15 noliteral Wrong package for constants
flush_cache.go 15:15 noliteral Wrong package for constants
flush_cache.go 24:16 noliteral Wrong package for constants
flush_cache.go 32:15 noliteral Wrong package for constants
gc.go 23:15 noliteral Wrong package for constants
gc.go 32:16 noliteral Wrong package for constants
gc.go 51:15 noliteral Wrong package for constants
gc.go 62:15 noliteral Wrong package for constants
healthcheck.go 17:15 noliteral Wrong package for constants
healthcheck.go 31:15 noliteral Wrong package for constants
list_shards.go 16:15 noliteral Wrong package for constants
list_shards.go 63:15 noliteral Wrong package for constants
set_netmap_status.go 17:15 noliteral Wrong package for constants
set_netmap_status.go 37:15 noliteral Wrong package for constants
set_netmap_status.go 48:15 noliteral Wrong package for constants
set_shard_mode.go 17:15 noliteral Wrong package for constants
set_shard_mode.go 36:15 noliteral Wrong package for constants
set_shard_mode.go 42:16 noliteral Wrong package for constants
set_shard_mode.go 55:15 noliteral Wrong package for constants
syncronize_tree.go 20:15 noliteral Wrong package for constants
syncronize_tree.go 24:15 noliteral Wrong package for constants
syncronize_tree.go 31:15 noliteral Wrong package for constants
syncronize_tree.go 36:15 noliteral Wrong package for constants
syncronize_tree.go 44:15 noliteral Wrong package for constants
control.go 54:5 noliteral Wrong package for constants
control.go 63:6 noliteral Wrong package for constants
control.go 108:6 noliteral Wrong package for constants
control.go 117:7 noliteral Wrong package for constants
Table of const check linter triggers <details> <summary>click</summary> | File | Line:Column | Error | Description | | --- | --- | --- | --- | | [assemble.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/object/get/assemble.go#L18) | 18:3 | noliteral | Wrong package for constants | | [get.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/object/get/get.go#L110) | 110:4 | noliteral | Wrong package for constants | | [calls.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/ir/server/calls.go#L19) | 19:15 | noliteral | Wrong package for constants | | [calls.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/ir/server/calls.go#L30) | 30:15 | noliteral | Wrong package for constants | | [calls.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/ir/server/calls.go#L41) | 41:15 | noliteral | Wrong package for constants | | [calls.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/ir/server/calls.go#L57) | 57:15 | noliteral | Wrong package for constants | | [calls.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/ir/server/calls.go#L68) | 68:15 | noliteral | Wrong package for constants | | [calls.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/ir/server/calls.go#L86) | 86:15 | noliteral | Wrong package for constants | | [calls.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/ir/server/calls.go#L89) | 89:15 | noliteral | Wrong package for constants | | [calls.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/ir/server/calls.go#L97) | 97:15 | noliteral | Wrong package for constants | | [convert.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/convert.go#L19) | 19:16 | noliteral | Wrong package for constants | | [doctor.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/doctor.go#L15) | 15:15 | noliteral | Wrong package for constants | | [doctor.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/doctor.go#L19) | 19:15 | noliteral | Wrong package for constants | | [doctor.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/doctor.go#L27) | 27:15 | noliteral | Wrong package for constants | | [doctor.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/doctor.go#L34) | 34:15 | noliteral | Wrong package for constants | | [evacuate.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate.go#L25) | 25:15 | noliteral | Wrong package for constants | | [evacuate.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate.go#L35) | 35:15 | noliteral | Wrong package for constants | | [evacuate.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate.go#L46) | 46:15 | noliteral | Wrong package for constants | | [evacuate_async.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate_async.go#L17) | 17:15 | noliteral | Wrong package for constants | | [evacuate_async.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate_async.go#L30) | 30:16 | noliteral | Wrong package for constants | | [evacuate_async.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate_async.go#L32) | 32:15 | noliteral | Wrong package for constants | | [evacuate_async.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate_async.go#L41) | 41:15 | noliteral | Wrong package for constants | | [evacuate_async.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate_async.go#L49) | 49:15 | noliteral | Wrong package for constants | | [evacuate_async.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate_async.go#L56) | 56:16 | noliteral | Wrong package for constants | | [evacuate_async.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate_async.go#L58) | 58:15 | noliteral | Wrong package for constants | | [evacuate_async.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate_async.go#L68) | 68:15 | noliteral | Wrong package for constants | | [evacuate_async.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate_async.go#L76) | 76:15 | noliteral | Wrong package for constants | | [evacuate_async.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate_async.go#L83) | 83:16 | noliteral | Wrong package for constants | | [evacuate_async.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate_async.go#L85) | 85:15 | noliteral | Wrong package for constants | | [evacuate_async.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/evacuate_async.go#L94) | 94:15 | noliteral | Wrong package for constants | | [flush_cache.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/flush_cache.go#L15) | 15:15 | noliteral | Wrong package for constants | | [flush_cache.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/flush_cache.go#L24) | 24:16 | noliteral | Wrong package for constants | | [flush_cache.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/flush_cache.go#L32) | 32:15 | noliteral | Wrong package for constants | | [gc.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/gc.go#L23) | 23:15 | noliteral | Wrong package for constants | | [gc.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/gc.go#L32) | 32:16 | noliteral | Wrong package for constants | | [gc.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/gc.go#L51) | 51:15 | noliteral | Wrong package for constants | | [gc.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/gc.go#L62) | 62:15 | noliteral | Wrong package for constants | | [healthcheck.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/healthcheck.go#L17) | 17:15 | noliteral | Wrong package for constants | | [healthcheck.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/healthcheck.go#L31) | 31:15 | noliteral | Wrong package for constants | | [list_shards.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/list_shards.go#L16) | 16:15 | noliteral | Wrong package for constants | | [list_shards.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/list_shards.go#L63) | 63:15 | noliteral | Wrong package for constants | | [set_netmap_status.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/set_netmap_status.go#L17) | 17:15 | noliteral | Wrong package for constants | | [set_netmap_status.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/set_netmap_status.go#L37) | 37:15 | noliteral | Wrong package for constants | | [set_netmap_status.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/set_netmap_status.go#L48) | 48:15 | noliteral | Wrong package for constants | | [set_shard_mode.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/set_shard_mode.go#L17) | 17:15 | noliteral | Wrong package for constants | | [set_shard_mode.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/set_shard_mode.go#L36) | 36:15 | noliteral | Wrong package for constants | | [set_shard_mode.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/set_shard_mode.go#L42) | 42:16 | noliteral | Wrong package for constants | | [set_shard_mode.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/set_shard_mode.go#L55) | 55:15 | noliteral | Wrong package for constants | | [syncronize_tree.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/syncronize_tree.go#L20) | 20:15 | noliteral | Wrong package for constants | | [syncronize_tree.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/syncronize_tree.go#L24) | 24:15 | noliteral | Wrong package for constants | | [syncronize_tree.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/syncronize_tree.go#L31) | 31:15 | noliteral | Wrong package for constants | | [syncronize_tree.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/syncronize_tree.go#L36) | 36:15 | noliteral | Wrong package for constants | | [syncronize_tree.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/services/control/server/syncronize_tree.go#L44) | 44:15 | noliteral | Wrong package for constants | | [control.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/local_object_storage/engine/control.go#L54) | 54:5 | noliteral | Wrong package for constants | | [control.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/local_object_storage/engine/control.go#L63) | 63:6 | noliteral | Wrong package for constants | | [control.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/local_object_storage/engine/control.go#L108) | 108:6 | noliteral | Wrong package for constants | | [control.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/65c72f3e0b0a2095625f6f7b278645bec7173199/pkg/local_object_storage/engine/control.go#L117) | 117:7 | noliteral | Wrong package for constants |
dstepanov-yadro approved these changes 2023-08-04 12:12:23 +00:00
achuprov force-pushed logs-linter from fbf106fb65 to a247b77934 2023-08-08 08:26:52 +00:00 Compare
achuprov force-pushed logs-linter from a247b77934 to b2873d488a 2023-08-08 08:29:47 +00:00 Compare
achuprov force-pushed logs-linter from b2873d488a to 74c77ea083 2023-08-08 08:47:15 +00:00 Compare
achuprov force-pushed logs-linter from 74c77ea083 to 979bf70580 2023-08-08 08:54:13 +00:00 Compare
achuprov force-pushed logs-linter from 979bf70580 to 9e13fd055a 2023-08-08 08:56:06 +00:00 Compare
acid-ant approved these changes 2023-08-08 10:25:41 +00:00
.golangci.yml Outdated
@ -40,0 +41,4 @@
noliteral:
path: bin/external_linters.so
original-url: git.frostfs.info/TrueCloudLab/linters.git
# settings:
acid-ant commented 2023-08-08 10:23:39 +00:00
Member
Copy link

It is better to avoid commented out code in RP, especially without comments.

It is better to avoid commented out code in RP, especially without comments.
fyrchik commented 2023-08-09 06:58:14 +00:00
Owner
Copy link
$ make lint-install
Cloning into 'linters'...
remote: Enumerating objects: 27, done.
remote: Counting objects: 100% (27/27), done.
remote: Compressing objects: 100% (23/23), done.
remote: Total 27 (delta 3), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (27/27), 41.87 KiB | 1.55 MiB/s, done.
Resolving deltas: 100% (3/3), done.
make[1]: Entering directory '/repo/frostfs/node/tmp/linters'
# command-line-arguments
loadinternal: cannot find runtime/cgo
make[1]: Leaving directory '/repo/frostfs/node/tmp/linters'
rmdir: failed to remove 'tmp': Directory not empty

2 things here:

  1. rmdir is probably not needed
  2. If we need CGO, lets set environment label explicitly, where needed.
``` $ make lint-install Cloning into 'linters'... remote: Enumerating objects: 27, done. remote: Counting objects: 100% (27/27), done. remote: Compressing objects: 100% (23/23), done. remote: Total 27 (delta 3), reused 0 (delta 0), pack-reused 0 Receiving objects: 100% (27/27), 41.87 KiB | 1.55 MiB/s, done. Resolving deltas: 100% (3/3), done. make[1]: Entering directory '/repo/frostfs/node/tmp/linters' # command-line-arguments loadinternal: cannot find runtime/cgo make[1]: Leaving directory '/repo/frostfs/node/tmp/linters' rmdir: failed to remove 'tmp': Directory not empty ``` 2 things here: 1. `rmdir` is probably not needed 2. If we need CGO, lets set environment label explicitly, where needed.
achuprov force-pushed logs-linter from 9e13fd055a to c0640191fb 2023-08-09 11:04:10 +00:00 Compare
achuprov commented 2023-08-09 11:11:42 +00:00
Author
Member
Copy link
$ make lint-install
Cloning into 'linters'...
remote: Enumerating objects: 27, done.
remote: Counting objects: 100% (27/27), done.
remote: Compressing objects: 100% (23/23), done.
remote: Total 27 (delta 3), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (27/27), 41.87 KiB | 1.55 MiB/s, done.
Resolving deltas: 100% (3/3), done.
make[1]: Entering directory '/repo/frostfs/node/tmp/linters'
# command-line-arguments
loadinternal: cannot find runtime/cgo
make[1]: Leaving directory '/repo/frostfs/node/tmp/linters'
rmdir: failed to remove 'tmp': Directory not empty

2 things here:

  1. rmdir is probably not needed
  2. If we need CGO, lets set environment label explicitly, where needed.

@fyrchik

  1. I delete the tmp directory only if it's empty. c0640191fb/Makefile (L147) I've added suppression of the output.
  2. I couldn't reproduce the error locally. If my solution doesn't help, please describe the environment and the output of 'go env'.
> ``` > $ make lint-install > Cloning into 'linters'... > remote: Enumerating objects: 27, done. > remote: Counting objects: 100% (27/27), done. > remote: Compressing objects: 100% (23/23), done. > remote: Total 27 (delta 3), reused 0 (delta 0), pack-reused 0 > Receiving objects: 100% (27/27), 41.87 KiB | 1.55 MiB/s, done. > Resolving deltas: 100% (3/3), done. > make[1]: Entering directory '/repo/frostfs/node/tmp/linters' > # command-line-arguments > loadinternal: cannot find runtime/cgo > make[1]: Leaving directory '/repo/frostfs/node/tmp/linters' > rmdir: failed to remove 'tmp': Directory not empty > > ``` > > 2 things here: > 1. `rmdir` is probably not needed > 2. If we need CGO, lets set environment label explicitly, where needed. @fyrchik 1. I delete the tmp directory only if it's empty. https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/c0640191fb8b1a4f8a292d171785843aaf676dbe/Makefile#L147 I've added suppression of the output. 2. I couldn't reproduce the error locally. If my solution doesn't help, please describe the environment and the output of 'go env'.
fyrchik reviewed 2023-08-09 11:30:53 +00:00
Makefile Outdated
@ -133,1 +135,4 @@
# Install linters
lint-install:
@mkdir -p tmp && \
fyrchik commented 2023-08-09 11:30:53 +00:00
Owner
Copy link

Some suggestions for this target:

  1. We can always fail unexpectedly, rm ./tmp/linters is a sane thing to do at the beginning.
  2. git clone can clone in a specific directory, no need to cd
  3. yes | rm can be replaced with -f flag for rm.
  4. Again, rm can remove arbitrary path, no need to cd.
  5. Same for make, see -C (it has a nice property of leaving the directory after the make)
  6. make can be replaced with $(MAKE), see https://www.gnu.org/software/make/manual/html_node/Recursion.html
  7. export CGO_ENABLED=1 is useful for 1 command (it does not affect go install -- I still have static binary on my machine. We can set it the same way as OUT_DIR
  8. tmp as a directory where we store stuff, can we a make variable (and I still believe .cache is better, as nothing is stored there manually, usually)
/r/f/node @c0640191fb• | 2 ❱ make lint-install && ldd ./bin/golangci-lint
...
        not a dynamic executable
Some suggestions for this target: 1. We can always fail unexpectedly, `rm ./tmp/linters` is a sane thing to do at the beginning. 2. `git clone` can clone in a specific directory, no need to `cd` 3. `yes | rm` can be replaced with `-f` flag for `rm`. 4. Again, `rm` can remove arbitrary path, no need to `cd`. 5. Same for make, see `-C` (it has a nice property of leaving the directory after the make) 6. `make` can be replaced with `$(MAKE)`, see https://www.gnu.org/software/make/manual/html_node/Recursion.html 7. `export CGO_ENABLED=1` is useful for 1 command (it does not affect `go install` -- I still have static binary on my machine. We can set it the same way as `OUT_DIR` 8. `tmp` as a directory where we store stuff, can we a make variable (and I still believe .cache is better, as nothing is stored there manually, usually) ``` /r/f/node @c0640191fb• | 2 ❱ make lint-install && ldd ./bin/golangci-lint ... not a dynamic executable ```
fyrchik commented 2023-08-09 11:32:53 +00:00
Owner
Copy link

For (6) and why it is good: if I use custom make (not in my PATH), with $(MAKE) the same binary will be used)
https://stackoverflow.com/questions/50510278/makefile-why-always-use-make-instead-of-make

For (6) and why it is good: if I use custom make (not in my PATH), with $(MAKE) _the same_ binary will be used) https://stackoverflow.com/questions/50510278/makefile-why-always-use-make-instead-of-make
fyrchik commented 2023-08-09 11:35:13 +00:00
Owner
Copy link

Also, from https://golangci-lint.run/contributing/new-linters/#how-to-add-a-private-linter-to-golangci-lint

This means that golangci-lint needs to be built for whatever machine you intend to run it on (cloning the golangci-lint repository and running a CGO_ENABLED=1 make build should do the trick for your machine).

Also, from https://golangci-lint.run/contributing/new-linters/#how-to-add-a-private-linter-to-golangci-lint >This means that golangci-lint needs to be built for whatever machine you intend to run it on (cloning the golangci-lint repository and running a CGO_ENABLED=1 make build should do the trick for your machine).
fyrchik commented 2023-08-09 11:41:08 +00:00
Owner
Copy link

@achuprov

I couldn't reproduce the error locally. If my solution doesn't help, please describe the environment and the output of 'go env'.

That's because I have CGO_ENABLED=0 explicitly set in my shell config (so I have my own defaults).

@achuprov > I couldn't reproduce the error locally. If my solution doesn't help, please describe the environment and the output of 'go env'. That's because I have `CGO_ENABLED=0` explicitly set in my shell config (so I have my own defaults).
achuprov force-pushed logs-linter from c0640191fb to e62e964b4f 2023-08-09 12:57:22 +00:00 Compare
achuprov changed title from Add logs-linter to WIP: Add logs-linter 2023-08-09 13:03:22 +00:00
achuprov force-pushed logs-linter from 4d7a8f29df to 0eea1901ed 2023-08-09 15:18:28 +00:00 Compare
achuprov force-pushed logs-linter from 0eea1901ed to 595cd57250 2023-08-09 15:22:10 +00:00 Compare
achuprov commented 2023-08-09 15:24:06 +00:00
Author
Member
Copy link

@fyrchik fixed

@fyrchik fixed
achuprov changed title from WIP: Add logs-linter to Add logs-linter 2023-08-09 15:26:47 +00:00
achuprov force-pushed logs-linter from 595cd57250 to ba225e5c74 2023-08-09 15:30:04 +00:00 Compare
fyrchik commented 2023-08-11 06:45:31 +00:00
Owner
Copy link
$ make lint-install lint
make: *** [Makefile:150: lint] Error 3
/r/f/node @ba225e5c74• | 2 ❱ make lint-install lint
Cloning into '.cache/linters'...
remote: Enumerating objects: 27, done.
remote: Counting objects: 100% (27/27), done.
remote: Compressing objects: 100% (23/23), done.
remote: Total 27 (delta 3), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (27/27), 7.68 KiB | 7.68 MiB/s, done.
Resolving deltas: 100% (3/3), done.
make[1]: Entering directory '/repo/frostfs/node/.cache/linters'
make[1]: Leaving directory '/repo/frostfs/node/.cache/linters'
/repo/frostfs/node/bin/golangci-lint --timeout=10m run
ERRO Unable to load custom analyzer noliteral:bin/external_linters.so, plugin: not implemented
ERRO Running error: unknown linters: 'noliteral', run 'golangci-lint help linters' to see the list of supported linters
make: *** [Makefile:150: lint] Error 3

Still doesn't work for me.

``` $ make lint-install lint make: *** [Makefile:150: lint] Error 3 /r/f/node @ba225e5c74• | 2 ❱ make lint-install lint Cloning into '.cache/linters'... remote: Enumerating objects: 27, done. remote: Counting objects: 100% (27/27), done. remote: Compressing objects: 100% (23/23), done. remote: Total 27 (delta 3), reused 0 (delta 0), pack-reused 0 Receiving objects: 100% (27/27), 7.68 KiB | 7.68 MiB/s, done. Resolving deltas: 100% (3/3), done. make[1]: Entering directory '/repo/frostfs/node/.cache/linters' make[1]: Leaving directory '/repo/frostfs/node/.cache/linters' /repo/frostfs/node/bin/golangci-lint --timeout=10m run ERRO Unable to load custom analyzer noliteral:bin/external_linters.so, plugin: not implemented ERRO Running error: unknown linters: 'noliteral', run 'golangci-lint help linters' to see the list of supported linters make: *** [Makefile:150: lint] Error 3 ``` Still doesn't work for me.
achuprov force-pushed logs-linter from ba225e5c74 to 9dce55821e 2023-08-11 07:46:29 +00:00 Compare
achuprov force-pushed logs-linter from 9dce55821e to c37eee6b75 2023-08-11 08:35:31 +00:00 Compare
achuprov commented 2023-08-11 08:44:13 +00:00
Author
Member
Copy link

@fyrchik I probably fixed the problem. But it might depend on your local environment. If this doesn't solve the issue, describe your environment and show the output of 'go env'."

@fyrchik I probably fixed the problem. But it might depend on your local environment. If this doesn't solve the issue, describe your environment and show the output of 'go env'."
fyrchik reviewed 2023-08-11 09:23:20 +00:00
.golangci.yml Outdated
@ -39,1 +39,4 @@
alias: objectSDK
custom:
noliteral:
path: bin/lint-1.54.0/external_linters.so
fyrchik commented 2023-08-11 09:21:43 +00:00
Owner
Copy link

Do we need to change it each time we update linter version? Can it be avoided?

Do we need to change it each time we update linter version? Can it be avoided?
Makefile Outdated
@ -134,0 +143,4 @@
@make -C $(TMP_DIR)/linters lib CGO_ENABLED=1 OUT_DIR=$(LINT_DIR)
@rm -rf $(TMP_DIR)/linters
@rmdir $(TMP_DIR) 2>/dev/null || true
@GOBIN=$(LINT_DIR) go install github.com/golangci/golangci-lint/cmd/golangci-lint@v$(LINT_VERSION)
fyrchik commented 2023-08-11 09:23:18 +00:00
Owner
Copy link

CGO_ENABLED=1 is what we miss here.

Cloning into '.cache/linters'...
remote: Enumerating objects: 27, done.
remote: Counting objects: 100% (27/27), done.
remote: Compressing objects: 100% (23/23), done.
remote: Total 27 (delta 3), reused 0 (delta 0), pack-reused 0
Receiving objects: 100% (27/27), 7.68 KiB | 7.68 MiB/s, done.
Resolving deltas: 100% (3/3), done.
make[1]: Entering directory '/repo/frostfs/node/.cache/linters'
make[1]: Leaving directory '/repo/frostfs/node/.cache/linters'
ERRO Unable to load custom analyzer noliteral:bin/lint-1.54.0/external_linters.so, plugin: not implemented
ERRO Running error: unknown linters: 'noliteral', run 'golangci-lint help linters' to see the list of supported linters
make: *** [Makefile:150: lint] Error 3
`CGO_ENABLED=1` is what we miss here. ``` Cloning into '.cache/linters'... remote: Enumerating objects: 27, done. remote: Counting objects: 100% (27/27), done. remote: Compressing objects: 100% (23/23), done. remote: Total 27 (delta 3), reused 0 (delta 0), pack-reused 0 Receiving objects: 100% (27/27), 7.68 KiB | 7.68 MiB/s, done. Resolving deltas: 100% (3/3), done. make[1]: Entering directory '/repo/frostfs/node/.cache/linters' make[1]: Leaving directory '/repo/frostfs/node/.cache/linters' ERRO Unable to load custom analyzer noliteral:bin/lint-1.54.0/external_linters.so, plugin: not implemented ERRO Running error: unknown linters: 'noliteral', run 'golangci-lint help linters' to see the list of supported linters make: *** [Makefile:150: lint] Error 3 ```
fyrchik reviewed 2023-08-11 09:24:30 +00:00
Makefile Outdated
@ -137,0 +152,4 @@
echo "Run make lint-install"; \
exit 1; \
fi
$(LINT_DIR)/golangci-lint --timeout=10m run
fyrchik commented 2023-08-11 09:24:30 +00:00
Owner
Copy link

Previously a timeout was different, whats the reason for change?
Anyway, we have it in the .golangci.yml, why do we need it here?

Previously a timeout was different, whats the reason for change? Anyway, we have it in the `.golangci.yml`, why do we need it here?
achuprov force-pushed logs-linter from c37eee6b75 to b5f94ec7d7 2023-08-11 10:05:54 +00:00 Compare
achuprov commented 2023-08-11 10:08:32 +00:00
Author
Member
Copy link

@fyrchik fixed

@fyrchik fixed
fyrchik approved these changes 2023-08-11 12:40:06 +00:00
fyrchik commented 2023-08-11 12:40:23 +00:00
Owner
Copy link

Please, rebase on the master branch.

Please, rebase on the master branch.
achuprov force-pushed logs-linter from b5f94ec7d7 to ad87493c41 2023-08-11 12:55:31 +00:00 Compare
achuprov commented 2023-08-11 12:58:26 +00:00
Author
Member
Copy link

@fyrchik Rebased

@fyrchik Rebased
dstepanov-yadro approved these changes 2023-08-11 13:55:42 +00:00
dstepanov-yadro scheduled this pull request to auto merge when all checks succeed 2023-08-11 13:56:01 +00:00
fyrchik merged commit ad87493c41 into master 2023-08-11 13:57:02 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
aarifullin
acid-ant
fyrchik
dstepanov-yadro
Labels
Clear labels   
P0

Highest

  
P1

High

  
P2

Medium

  
P3

Low

  
badger

Badger DB related issues

  
frostfs-adm

   
frostfs-cli

   
frostfs-ir

   
frostfs-lens

   
frostfs-node

   
good first issue

Good for newcomers

  
triage
  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff

  
blocked

The issue or PR is blocked by something

  
bug

Something is not working

  
config

Configuration format update or breaking change

  
discussion

Talking about something in order to reach a decision or to exchange ideas

  
documentation

Documentation related

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
go

Language features adoption

  
help wanted

Extra attention is needed

  
internal

   
invalid

Something is wrong

  
kludge

This is a kludge and we know it

  
observability

Related to metrics, tracing and logs

  
perfomance

Perfomance related

  
question

More information is needed

  
refactoring

Refactoring

  
wontfix

This won't be fixed

No labels
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
5 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#526
No description provided.
Delete branch "achuprov/frostfs-node:logs-linter"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?