Skip to content
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

Misleading error message when creating notifications while specifying a notification ID. #1289

Closed
JordanW1300 opened this issue Jun 16, 2024 · 1 comment · Fixed by #1290 or #1291
Closed
Assignees
Labels
api: storage Issues related to the googleapis/python-storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@JordanW1300
Copy link

Issue Summary:
When following the Google documentation [1] but modifying the code to include a notification ID on the notification object before creating it you get an error stating that the notification ID exists. However when running the gcloud command to describe a notification with that same ID it actually does not exist.

When looking at the source for this method we can see this error message is returned anytime the notification ID on the notification object is set to anything other than None [2].

The reference documentation [3] also states that the notification ID is set by the server. This suggests this is working as intended and the creation of notifications when the ID is set to a value other than none is not allowed.

My Ask:
Is there a specific reason why the error message states that a notification already exists with the ID?

This can be misleading and seems to imply that it is possible to create a notification with a custom ID however the one selected is already in use.

Possible Change:
This error message is returned directly within the create method not from a method/function called within it. I would think it would be more informative to return a message stating that the notification ID must be set to None in order to create a new notification. Unless of course there is a specific reason for this message.

[1] https://cloud.google.com/storage/docs/reporting-changes#storage_create_bucket_notifications-python
[2]

if self.notification_id is not None:

[3] https://cloud.google.com/python/docs/reference/storage/1.44.0/google.cloud.storage.notification.BucketNotification#google_cloud_storage_notification_BucketNotification_notification_id

Steps to reproduce

  1. Follow GCP tutorial using code snippet
  2. Modify it to specify a notification ID on the notification object before it is created
  3. Run the program and receive the error below.

Code example

*** Project, bucket, and topic names redacted for privacy ***

from google.cloud import storage

def create_bucket_notifications(bucket_name, topic_name, notification_id=None):

    storage_client = storage.Client('PROJECT-ID')
    bucket = storage_client.bucket(bucket_name)
    notification = bucket.notification(topic_name=topic_name, notification_id=notification_id)
    notification.create()

    print(f"Successfully created notification with ID {notification.notification_id} for bucket {bucket_name}")

create_bucket_notifications('MY-BUCKET', 'MY-TOPIC', 15)

Stack trace

*** Project, bucket, topic, and system user names redacted for privacy ***

Traceback (most recent call last):
File "/home/user/bucket-notification/text.py", line 12, in
create_bucket_notifications("MY-BUCKET", 'MY-TOPIC', 15)
File "/home/wriker/bucket-notification/text.py", line 8, in create_bucket_notifications
notification.create()
File "/usr/local/lib/python3.10/dist-packages/google/cloud/storage/notification.py", line 257, in create
raise ValueError(
ValueError: Notification already exists w/ id: 15

@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/python-storage API. label Jun 16, 2024
@tritone tritone added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Jun 17, 2024
@tritone tritone assigned tritone and unassigned BrennaEpp Jun 17, 2024
tritone added a commit to tritone/python-storage that referenced this issue Jun 17, 2024
To create a new notification, the notification_id field must be
set to None. Update the error message to clarify this.

Fixes googleapis#1289
@tritone
Copy link
Contributor

tritone commented Jun 17, 2024

Thanks for the issue. I think the reason for the error message being what it is currently is that you could run into this issue if you called create twice on the same notification object. However, you're correct that it could also happen by setting the notification_id kwarg. I have made a PR to clarify.

tritone added a commit that referenced this issue Jun 17, 2024
To create a new notification, the notification_id field must be
set to None. Update the error message to clarify this.

Fixes #1289
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/python-storage API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
3 participants