refactor: bump QPC_MIN_SERVER_VERSION to 2.5.0#407
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideCentralizes the minimum supported server version for all CLI commands on the global QPC_MIN_SERVER_VERSION constant and bumps it from 2.4.0 to 2.5.0, removing legacy per-command overrides in specific report subcommands. Class diagram for centralized QPC_MIN_SERVER_VERSION usageclassDiagram
class CliCommand {
- req_headers
- response
- min_server_version : string
+ _validate_args()
}
class ReportDeploymentsCommand {
- report_id
+ _validate_args()
}
class ReportDetailsCommand {
- report_id
+ _validate_args()
}
class ReportDownloadCommand {
- report_id
+ _validate_args()
}
class ReportInsightsCommand {
- report_id
+ _insights_report_available(sources)
}
CliCommand <|-- ReportDeploymentsCommand
CliCommand <|-- ReportDetailsCommand
CliCommand <|-- ReportDownloadCommand
CliCommand <|-- ReportInsightsCommand
class QpcUtils {
+ QPC_MIN_SERVER_VERSION : string = "2.5.0"
}
QpcUtils ..> CliCommand : provides_min_server_version
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Now that min_server_version is centralized, scan for any remaining hard-coded server version strings in user-facing error messages or logic and replace them with QPC_MIN_SERVER_VERSION to avoid future drift.
- Consider tightening the new comment on min_server_version to explicitly mention that any API changes requiring a newer server must be accompanied by a QPC_MIN_SERVER_VERSION bump, to preserve the previous guidance that was removed from CliCommand.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Now that min_server_version is centralized, scan for any remaining hard-coded server version strings in user-facing error messages or logic and replace them with QPC_MIN_SERVER_VERSION to avoid future drift.
- Consider tightening the new comment on min_server_version to explicitly mention that any API changes requiring a newer server must be accompanied by a QPC_MIN_SERVER_VERSION bump, to preserve the previous guidance that was removed from CliCommand.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
mirekdlugosz
left a comment
There was a problem hiding this comment.
I don't understand what is happening in unit tests, but I think you should also remove all headers={"X-Server-Version": VERSION} from mocks (example).
I see only tests in qpc/tests/report/ set that, and it so happens that only tests in qpc/tests/report/ are failing CI. Although I have not checked that all failing tests have that headers set.
Also, personally I would not include QPC_MIN_SERVER_VERSION bump here. We routinely do that as part of new version release. Yes, I know jira ticket explicitly says that. I still think jira ticket mixes two different things and forces you to create a PR that can't be merged due to CI failures.
4a6ab40 to
5e28cda
Compare
…e current server version in mock headers
5e28cda to
56521e2
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #407 +/- ##
==========================================
- Coverage 96.04% 96.04% -0.01%
==========================================
Files 122 122
Lines 7772 7762 -10
==========================================
- Hits 7465 7455 -10
Misses 307 307 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Remove per-subcommand min_server_version overrides from report deployments, details, download, and insights commands. Mixing old and new server/client is not supported, so all commands should use the single global QPC_MIN_SERVER_VERSION. Bump it from 2.4.0 to 2.5.0 to reflect the new APIs required by report show and list. Relates to JIRA: DISCOVERY-1272
3c12d2a to
70f1a26
Compare
Remove per-subcommand min_server_version overrides from report deployments, details, download, and insights commands. Mixing old and new server/client is not supported, so all commands should use the single global QPC_MIN_SERVER_VERSION. Bump it from 2.4.0 to 2.5.0 to reflect the new APIs required by report show and list.
Relates to JIRA: DISCOVERY-1272
Summary by Sourcery
Align CLI minimum server version requirements with a single global value and update it to 2.5.0.
Enhancements: