Skip to content
This repository was archived by the owner on Feb 8, 2024. It is now read-only.

Conversation

@sjain09
Copy link
Contributor

@sjain09 sjain09 commented Sep 13, 2022

Signed-off-by: Saurabh Jain [email protected]

Problem Statement

Updated wrong values for SVC_INITIAL_STARTUP_MEM, SVC_MEM_PER_THREAD_PER_REQ constants in PR #131.

Coding

Checklist for Author

  • Coding conventions are followed and code is consistent

Testing

Checklist for Author

  • Unit and System Tests are added
  • Test Cases cover Happy Path, Non-Happy Path and Scalability
  • Testing was performed with RPM

Impact Analysis

Checklist for Author/Reviewer/GateKeeper

  • Interface change (if any) are documented
  • Side effects on other features (deployment/upgrade)
  • Dependencies on other component(s)

Review Checklist

Checklist for Author

  • JIRA number/GitHub Issue added to PR
  • PR is self reviewed
  • Jira and state/status is updated and JIRA is updated with PR link
  • Check if the description is clear and explained

Documentation

Checklist for Author

  • Changes done to WIKI / Confluence page / Quick Start Guide

@sjain09 sjain09 requested a review from cdeshmukh as a code owner September 13, 2022 14:10
@cla-bot cla-bot bot added the cla-signed label Sep 13, 2022
Copy link
Contributor

@cdeshmukh cdeshmukh left a comment

Choose a reason for hiding this comment

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

@sjain09 conflicting changes in subsequent PRs make me wonder how testing being done for the these PRs, previous ones are pretty much corrections in typo (#PR 128) or wrong values ( PR 131, which this is correcting now).

Do link some test details and state how these changed values are impacting.

@cdeshmukh cdeshmukh self-requested a review September 13, 2022 14:46
@sachitanands
Copy link
Contributor

sachitanands commented Sep 13, 2022

Tested deployment with these changes with local docker image,
image

@sjain09
Copy link
Contributor Author

sjain09 commented Sep 13, 2022

This change is unit tested on local setup. The values are now reflecting in rgw config file as expected.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants