-
Notifications
You must be signed in to change notification settings - Fork 1
[WIP] Fixing drs list #146
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
Conversation
…lso reverting the logging to fix debug output issue
|
Summary Token refresh now triggers a new token request when the access token is expired or when the expiration claim can’t be parsed, ensuring tokens are refreshed even on parsing errors. Logging was simplified to use the standard library *log.Logger, removing the custom thread-safe wrapper and providing a global logger plus a no-op logger for tests/fallbacks. Logger usage across DRS components was updated to accept *log.Logger (e.g., remote client wiring and DRS map operations), and indexd page parsing now logs the response body alongside unmarshal errors for better diagnostics. |
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.
Pull request overview
This PR simplifies the logging infrastructure by replacing the custom drslog.Logger wrapper with Go's standard log.Logger. The custom wrapper previously added thread-safety with mutex locks, but this is unnecessary since log.Logger is already thread-safe.
Changes:
- Removed custom
drslog.Loggertype and its mutex-based thread-safety wrappers - Updated all function signatures across the codebase to use
*log.Loggerinstead of*drslog.Logger - Improved test reliability by using
NewNoOpLogger()instead ofGetLogger()in tests - Enhanced error logging in
ListObjectsto include response body for debugging
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| drslog/logger.go | Simplified logger by removing custom wrapper and mutex-based thread-safety |
| s3_utils/s3.go | Updated type signatures to use standard log.Logger |
| drsmap/drs_map.go | Updated function signatures to use standard log.Logger |
| drs/store.go | Updated function signatures to use standard log.Logger |
| config/config.go | Updated interface and method signatures to use standard log.Logger |
| cmd/remote/add/gen3.go | Updated function signature to use standard log.Logger |
| cmd/remote/add/anvil.go | Updated function signature to use standard log.Logger |
| cmd/initialize/main.go | Updated function signature to use standard log.Logger |
| client/indexd/tests/test_helpers_test.go | Improved test isolation by using NewNoOpLogger() |
| client/indexd/tests/client_read_test.go | Improved test isolation by using NewNoOpLogger() |
| client/indexd/indexd_client.go | Enhanced error message and updated type signature |
| client/indexd/gen3_remote.go | Updated method signature to use standard log.Logger |
| client/indexd/auth_handler.go | Modified token refresh logic and has spelling error |
| client/indexd/add_url.go | Updated function signatures to use standard log.Logger |
| client/anvil/remote.go | Updated method signature to use standard log.Logger |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.