unused message arg in Batch.will_accept()

@pradn

Looks like message arg is never used by will_accept() method:

python-pubsub/google/cloud/pubsub_v1/publisher/_batch/base.py

Lines 112 to 136 in cf9e87c

    
               def will_accept(self, message): 
        
                   """Return True if the batch is able to accept the message. 
        
                   In concurrent implementations, the attributes on the current batch 
        
                   may be modified by other workers. With this in mind, the caller will 
        
                   likely want to hold a lock that will make sure the state remains 
        
                   the same after the "will accept?" question is answered. 
        
                   Args: 
        
                       message (~.pubsub_v1.types.PubsubMessage): The Pub/Sub message. 
        
                   Returns: 
        
                       bool: Whether this batch can accept the message. 
        
                   """ 
        
                   # If this batch is not accepting messages generally, return False. 
        
                   if self.status != BatchStatus.ACCEPTING_MESSAGES: 
        
                       return False 
        
                   # If this message will make the batch exceed the ``max_messages`` 
        
                   # setting, return False. 
        
                   if len(self.messages) >= self.settings.max_messages: 
        
                       return False 
        
                   # Okay, everything is good. 
        
                   return True

Most likely it should be deleted to avoid any confusion, but I suppose it should be done with deprecation. Batch() is not something pretending to be manually used by users, but who knows.

@pradn, please, comment

will_accept is unnecessary, and partially duplicates the checks done in thread.Batch.publish. We should just incorporate it into publish().

I don't think people rely on being able to override Batch. We have made a bunch of changes to Batch that they would have to keep up with, so it's not advisable anyway. So it should be fine to just modify the interface without deprecation.

Navigation Menu

Repository navigation

unused message arg in Batch.will_accept() #105

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	def will_accept(self, message):
	"""Return True if the batch is able to accept the message.

	In concurrent implementations, the attributes on the current batch
	may be modified by other workers. With this in mind, the caller will
	likely want to hold a lock that will make sure the state remains
	the same after the "will accept?" question is answered.

	Args:
	message (~.pubsub_v1.types.PubsubMessage): The Pub/Sub message.

	Returns:
	bool: Whether this batch can accept the message.
	"""
	# If this batch is not accepting messages generally, return False.
	if self.status != BatchStatus.ACCEPTING_MESSAGES:
	return False

	# If this message will make the batch exceed the ``max_messages``
	# setting, return False.
	if len(self.messages) >= self.settings.max_messages:
	return False

	# Okay, everything is good.
	return True

unused message arg in Batch.will_accept() #105

Description

Activity

pradn commented on May 27, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions