-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
chore(license): default to basic plan if license validation fails #6972
Conversation
contributes to - https://github.com/SigNoz/platform-pod/issues/343 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Reviewed everything up to 4257ec2 in 47 seconds
More details
- Looked at
405
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. ee/query-service/license/sqlite/init.go:12
- Draft comment:
The error message should refer to the 'sites' table instead of 'licenses' table for clarity.
return fmt.Errorf("error in creating sites table: %s", err.Error())
- Reason this comment was not posted:
Confidence changes required:50%
The error message in the InitDB function in init.go is misleading. It mentions 'licenses table' instead of 'sites table'. This could cause confusion during debugging.
2. ee/query-service/model/license.go:1
- Draft comment:
Avoid using thecomponent/index.tsx
file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This is applicable in other files as well. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_IhMU1jXiYEPMRrp0
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 57bc71e in 13 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. ee/query-service/license/manager.go:225
- Draft comment:
Consider clarifying the comment to indicate that the failed attempts counter is reset only when validation is successful. - Reason this comment was not posted:
Confidence changes required:20%
The logic for resetting the failed attempts counter is correct, but the comment can be improved for clarity.
Workflow ID: wflow_anStyCZGgmaG7Ocm
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 31fd03b in 25 seconds
More details
- Looked at
23
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. ee/query-service/license/sqlite/init.go:15
- Draft comment:
Thelicenses_v3
table appears redundant with the newlicenses
table. Consider removing it if it's no longer needed. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
2. ee/query-service/license/sqlite/init.go:15
- Draft comment:
Avoid using inline styles in React components. Instead, use external stylesheets, CSS classes, or styled components. This comment applies to React components, not Go code. - Reason this comment was not posted:
Confidence changes required:0%
The code uses inline SQL schema definitions, which is acceptable in this context. However, there are no violations of the specific rules provided.
Workflow ID: wflow_VP36uhNk6yDhEM7S
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 1844dde in 14 seconds
More details
- Looked at
23
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_W0Yvb8JDtVd8QrCb
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Incremental review on dfa89b8 in 41 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. ee/query-service/license/manager.go:219
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values in log statements. This applies to the log statement on line 219 and elsewhere if applicable. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_ejkHVbaSp5pGQL5b
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 24016f3 in 29 seconds
More details
- Looked at
20
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. ee/query-service/license/manager.go:212
- Draft comment:
The error log message is duplicated. Consider removing the first occurrence to avoid redundancy. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_8hMKqQzBjSgjA3YI
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on 5e09270 in 35 seconds
More details
- Looked at
12
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. ee/query-service/license/manager.go:220
- Draft comment:
Consider checking if thelm.done
channel is already closed before writing to it to avoid potential panics. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment raises a valid concern about potential panics when writing to a closed channel. However, analyzing the code flow: 1) The channel is only closed in Stop() 2) After Stop(), ValidatorV3 returns immediately 3) ValidateV3 is only called from ValidatorV3 4) So if the channel is closed, ValidatorV3 would have already returned and ValidateV3 wouldn't be running. The code appears safe as is.
I could be missing some edge case where ValidateV3 is called directly from somewhere else. Also, there could be race conditions I'm not seeing.
Looking at the code again, ValidateV3 is only called from ValidatorV3 or directly for initial validation. The mutex lock in ValidateV3 prevents race conditions with Stop().
The comment should be deleted. The code is actually safe because the mutex and control flow prevent the scenario where we would write to a closed channel.
Workflow ID: wflow_daD04ul3apWXcw5g
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me! Incremental review on b4875e0 in 22 seconds
More details
- Looked at
12
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. ee/query-service/license/manager.go:220
- Draft comment:
Usinglm.done <- struct{}{}
to signal thedone
channel can cause a panic if the channel is already closed. Consider using a select statement to check if the channel is closed before sending to it. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_1PhSXkJnxIcK97aF
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Summary