Skip to content

Conversation

@bartekpacia
Copy link
Owner

@bartekpacia bartekpacia commented Apr 17, 2025

fix #37

@bartekpacia bartekpacia force-pushed the feature/user_config_cache branch from ce21e3e to 510bae9 Compare April 17, 2025 20:47
@bartekpacia bartekpacia requested a review from Copilot April 17, 2025 20:48
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@bartekpacia bartekpacia requested a review from Copilot April 17, 2025 20:48
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a caching mechanism for the user configuration and refactors logging setup across commands. Key changes include:

  • Adding a debug log to track resource session openings in highlevel/connect.go.
  • Refactoring slog logging configuration in cmd/fhome/main.go based on environment variables and CLI flags.
  • Replacing synchronous client creation with a cached approach in cmd/fhome/commands.go and adding a new caching implementation in cmd/fhome/cache.go.

Reviewed Changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated 1 comment.

File Description
highlevel/connect.go Updated comment and added a debug log when opening a resource session
cmd/fhome/main.go Revised logger configuration to use environment and command-line flags
cmd/fhome/commands.go Updated ShellComplete to use the new getUserConfig function
cmd/fhome/cache.go Added new caching logic for user configuration with background updates
Files not reviewed (2)
  • .idea/dictionaries/project.xml: Language not supported
  • .idea/inspectionProfiles/Project_Default.xml: Language not supported
Comments suppressed due to low confidence (1)

cmd/fhome/commands.go:323

  • Using panic for error handling in ShellComplete may abruptly terminate the CLI process; consider handling the error gracefully instead.
if err != nil { panic(err) }

slog.String("type", myResources.ResourceType0),
)

slog.Debug("opening client to resource session")
Copy link

Copilot AI Apr 17, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider including additional context details (e.g., resource identifiers) in the debug log to aid troubleshooting.

Suggested change
slog.Debug("opening client to resource session")
slog.Debug("opening client to resource session", slog.String("resourcePassword", config.ResourcePassword))

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

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

XDD

@bartekpacia bartekpacia merged commit 7aa1bd0 into master Apr 17, 2025
1 check passed
@bartekpacia bartekpacia deleted the feature/user_config_cache branch April 17, 2025 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shell completion is slow

2 participants