Skip to content

Commit

Permalink
fix: mutable defaults in init (#48)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mercy811 committed Aug 3, 2023
1 parent 0c8c6a4 commit 958e394
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 5 deletions.
4 changes: 2 additions & 2 deletions src/amplitude/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ class Amplitude:
shutdown(): Shutdown the client instance
"""

def __init__(self, api_key: str, configuration: Config = Config()):
def __init__(self, api_key: str, configuration: Optional[Config] = None):
"""The constructor for the Amplitude class
Args:
api_key (str): The api key of amplitude project. Must be set properly before tracking events.
configuration (amplitude.config.Config, optional): The configuration of client instance. A new instance
with default config value will be used by default.
"""
self.configuration: Config = configuration
self.configuration: Config = configuration or Config()
self.configuration.api_key = api_key
self.__timeline = Timeline()
self.__timeline.setup(self)
Expand Down
4 changes: 2 additions & 2 deletions src/amplitude/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def __init__(self, api_key: str = None,
server_zone: str = constants.DEFAULT_ZONE,
use_batch: bool = False,
server_url: Optional[str] = None,
storage_provider: StorageProvider = InMemoryStorageProvider(),
storage_provider: Optional[StorageProvider] = None,
plan: Plan = None,
ingestion_metadata: IngestionMetadata = None):
"""The constructor of Config class"""
Expand All @@ -70,7 +70,7 @@ def __init__(self, api_key: str = None,
self.server_zone: str = server_zone
self.use_batch: bool = use_batch
self._url: Optional[str] = server_url
self.storage_provider: StorageProvider = storage_provider
self.storage_provider: StorageProvider = storage_provider or InMemoryStorageProvider()
self.opt_out: bool = False
self.plan: Plan = plan
self.ingestion_metadata: IngestionMetadata = ingestion_metadata
Expand Down
7 changes: 7 additions & 0 deletions src/test/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ def setUp(self) -> None:

def tearDown(self) -> None:
self.client.shutdown()

def test_amplitude_client_init_multiple_instance(self):
# test multiple instance inited with different API keys
client1 = Amplitude(api_key="test api key 1")
client2 = Amplitude(api_key="test api key 2")
self.assertNotEqual(client1.configuration.api_key, client2.configuration.api_key)
self.assertNotEqual(client1.configuration.storage_provider, client2.configuration.storage_provider)

def test_amplitude_client_track_success(self):
post_method = MagicMock()
Expand Down
6 changes: 5 additions & 1 deletion src/test/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,11 @@ def test_config_flush_queue_size_decrease_no_less_than_one(self):
self.assertEqual(1, config.flush_queue_size)
config._increase_flush_divider()
self.assertEqual(1, config.flush_queue_size)


def test_config_multiple_instances_has_different_storage_provider(self):
config1 = Config()
config2 = Config()
self.assertIsNot(config1.storage_provider, config2.storage_provider)

if __name__ == '__main__':
unittest.main()

0 comments on commit 958e394

Please sign in to comment.