[feature/PI-631] Generate Product ids #429
Conversation
jameslinnell
force-pushed
the
feature/PI-631-generate_product_ids
branch
from
3519854 to
861e56e
Compare
There was a problem hiding this comment.
Super minor but probably call this file product_id_generator.py just so we don't get confused with other ids :)
There was a problem hiding this comment.
It would if we wanted in the future be able to generate other ids, it's just that by default it generates product_ids. So it is generic
| for _ in range(id_count): | ||
| saved = False | ||
| while not saved: | ||
| generated_id = generator() | ||
| saved = save_to_set(generated_id, ids) | ||
| ids = saved |
There was a problem hiding this comment.
maybe to make more concise:
| for _ in range(id_count): | |
| saved = False | |
| while not saved: | |
| generated_id = generator() | |
| saved = save_to_set(generated_id, ids) | |
| ids = saved | |
| initial_len = len(ids) | |
| while len(ids) - initial_len < id_count: | |
| ids.add(generator()) |
| generated_product_ids = set() | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module", autouse=True) | ||
| def _get_generated_ids(): | ||
| global generated_product_ids |
There was a problem hiding this comment.
| generated_product_ids = set() | |
| @pytest.fixture(scope="module", autouse=True) | |
| def _get_generated_ids(): | |
| global generated_product_ids | |
| generated_product_ids = set() | |
| @pytest.fixture(scope="module") | |
| def generated_product_ids(): |
and then use the generated_product_ids fixture in the arg of the required test?
There was a problem hiding this comment.
Either way the fixture is called once, but ok.
| def create(cls): | ||
| """No current_id needed, key is generated randomly.""" | ||
| rng = random.Random(datetime.now().timestamp()) | ||
| product_id = "-".join( | ||
| "".join(rng.choices(PRODUCT_ID_VALID_CHARS, k=PRODUCT_ID_PART_LENGTH)) | ||
| for _ in range(PRODUCT_ID_NUMBER_OF_PARTS) | ||
| ) | ||
| return cls(__root__=f"P.{product_id}") | ||
| existing_ids = cls.load_existing_ids() | ||
| while True: | ||
| """No current_id needed, key is generated randomly.""" | ||
| rng = random.Random(datetime.now().timestamp()) | ||
| product_id = "-".join( | ||
| "".join(rng.choices(PRODUCT_ID_VALID_CHARS, k=PRODUCT_ID_PART_LENGTH)) | ||
| for _ in range(PRODUCT_ID_NUMBER_OF_PARTS) | ||
| ) | ||
| if f"P.{product_id}" not in existing_ids: | ||
| return cls(__root__=f"P.{product_id}") |
There was a problem hiding this comment.
to minimise the change, perhaps could leave the entire method unchanged instead could just change the return line of the function to:
if f"P.{product_id}" in cls.load_existing_ids():
return cls.create()
return cls(__root__=f"P.{product_id}")
There was a problem hiding this comment.
I did it this way so that the file is called once. In your suggestion the file would get called on each recursive flow. Your way seems less efficient
There was a problem hiding this comment.
i'd cache the load under the hood to avoid loading the file twice, so e.g.
@cache
def _load_existing_ids():
# load the ids
...
class CantRememberTheNameOfTheClass:
...
@classmethod
def load_existing_ids(cls):
return _load_existing_ids()
...
jameslinnell
force-pushed
the
feature/PI-631-generate_product_ids
branch
from
adb10ab to
7c0e54f
Compare
No description provided.