Skip to content
This repository was archived by the owner on Sep 26, 2025. It is now read-only.

chore: mock empty app state to downsize postgres storage capacity#955

Merged
dbm03 merged 7 commits into
mainfrom
fix/mock-app-state-downsize-postgres
Apr 23, 2025
Merged

chore: mock empty app state to downsize postgres storage capacity#955
dbm03 merged 7 commits into
mainfrom
fix/mock-app-state-downsize-postgres

Conversation

@dbm03
Copy link
Copy Markdown
Member

@dbm03 dbm03 commented Apr 13, 2025

PR Type

Enhancement, Bug fix


Description

  • Mock app state based on caller function

  • Return paused state (6) by default

  • Return running state (5) for database version change

  • Implement runtime caller inspection for state determination


Changes walkthrough 📝

Relevant files
Enhancement
querier.go
Implement dynamic app state mocking based on caller           

cmd/configserver/querier.go

  • Added runtime package import and caller inspection
  • Implemented logic to return different states based on caller
  • Default to paused state (6) for most cases
  • Return running state (5) for database version change validation
  • +19/-1   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Copy Markdown
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Hardcoded Values

    The function uses hardcoded values (5 and 6) for app states. Consider using named constants for better readability and maintainability.

    		return 5, nil
    	}
    	if !more {
    		break
    	}
    }
    
    // Default to appPaused for all other cases
    return 6, nil //nolint:mnd
    Error Handling

    The function doesn't handle potential errors from runtime.Callers or frames.Next(). Consider adding error checks and handling.

    n := runtime.Callers(2, pc)
    frames := runtime.CallersFrames(pc[:n])
    
    for {
    	frame, more := frames.Next()
    	if strings.Contains(frame.Function, "changeDatabaseVersionValidate") {
    		return 5, nil
    	}
    	if !more {
    		break
    	}
    }

    @github-actions
    Copy link
    Copy Markdown
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Use constants for magic numbers

    Consider using constants for the magic numbers 5 and 6 to improve code readability
    and maintainability. Define these constants with meaningful names at the package
    level.

    cmd/configserver/querier.go [15-30]

    +const (
    +    appRunningState = 5
    +    appPausedState  = 6
    +)
    +
     pc := make([]uintptr, 10)
     n := runtime.Callers(2, pc)
     frames := runtime.CallersFrames(pc[:n])
     
     for {
         frame, more := frames.Next()
         if strings.Contains(frame.Function, "changeDatabaseVersionValidate") {
    -        return 5, nil
    +        return appRunningState, nil
         }
         if !more {
             break
         }
     }
     
     // Default to appPaused for all other cases
    -return 6, nil //nolint:mnd
    +return appPausedState, nil
    Suggestion importance[1-10]: 7

    __

    Why: Using constants for magic numbers (5 and 6) improves code readability and maintainability. This suggestion enhances code quality by making the meaning of these values clearer and easier to update if needed.

    Medium

    @dbarrosop
    Copy link
    Copy Markdown
    Member

    dbarrosop commented Apr 14, 2025

    I appreciate this PR's ingenuity :P but I suspect this is going to haunt us forever. I think it might be better to just return state 0 (there is no state as there is no app) and treat state 0 accordingly where needed.

    @dbm03 dbm03 force-pushed the fix/mock-app-state-downsize-postgres branch from 983962d to a0a0c6a Compare April 14, 2025 12:50
    @dbm03 dbm03 changed the title chore: mock paused app state to downsize postgres storage capacity chore: mock empty app state to downsize postgres storage capacity Apr 14, 2025
    @dbm03 dbm03 force-pushed the fix/mock-app-state-downsize-postgres branch from d714b52 to a0a0c6a Compare April 14, 2025 13:07
    @dbarrosop dbarrosop changed the title chore: mock empty app state to downsize postgres storage capacity chore: update schema Apr 23, 2025
    @dbarrosop dbarrosop changed the title chore: update schema chore: mock empty app state to downsize postgres storage capacity Apr 23, 2025
    @dbm03 dbm03 merged commit d841a6f into main Apr 23, 2025
    8 checks passed
    @dbm03 dbm03 deleted the fix/mock-app-state-downsize-postgres branch April 23, 2025 11:31
    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.

    2 participants