Revert "Update to latest spanner package (#1781)"#1784
Revert "Update to latest spanner package (#1781)"#1784
Conversation
This reverts commit c027143.
Summary of ChangesHello @n-h-diaz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request performs a full revert of a previous dependency update, specifically targeting the Spanner package and other Go modules. The rollback is a temporary measure to resolve build and deployment failures that arose because the original update lacked crucial Docker configuration changes. The author plans to re-implement the dependency updates, ensuring all necessary accompanying changes are included, in a future pull request. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request reverts the update to the latest spanner package due to issues with the docker image update. The changes primarily involve downgrading several Go packages in go.mod and reverting the PubSub client library update in internal/pubsub/pubsub.go and internal/server/server.go.
| if err != nil { | ||
| return fmt.Errorf("failed to create subscription: %w", err) | ||
| return err |
There was a problem hiding this comment.
high: The error err is returned without being wrapped with a user-friendly message. Wrap the error with a descriptive message to provide more context to the user, as per the general rules.
References
- Wrap generic errors from underlying functions with more descriptive, user-friendly error messages.
| err := client.SubscriptionAdminClient.DeleteSubscription(ctx, &pubsubpb.DeleteSubscriptionRequest{ | ||
| Subscription: subscription.GetName(), | ||
| }) | ||
| err := subscriber.Delete(ctx) |
There was a problem hiding this comment.
high: The error err is returned without being wrapped with a user-friendly message. Wrap the error with a descriptive message to provide more context to the user, as per the general rules.
References
- Wrap generic errors from underlying functions with more descriptive, user-friendly error messages.
| expiration, _ := time.ParseDuration("36h") | ||
| retention, _ := time.ParseDuration("24h") |
There was a problem hiding this comment.
medium: The code ignores the error returned by time.ParseDuration. While the durations are hardcoded and unlikely to cause an error, it's still best practice to handle the error to prevent unexpected behavior. Consider logging the error or using a more robust error handling mechanism.
1 participant
This reverts commit c027143.
The original PR was missing updates to docker to also update the version. This is causing the mixer-on-push-to-branch to autopush to fail. Rolling back so I can fix the issue within the same PR