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.

Sign up for GitHub

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

Jump to bottom

Use session-scoped inventory by default #55

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+66 −28
Open

Use session-scoped inventory by default #55

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6352bbe
Use session-scoped inventory by default
synkd Nov 22, 2024
89488f9
Get unit tests into passing state
synkd Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion manifester/commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ def inventory(details, sync, offline_token):
border = "-" * 38
if sync:
helpers.update_inventory(
Manifester(minimal_init=True, offline_token=offline_token).subscription_allocations
Manifester(minimal_init=True, offline_token=offline_token).subscription_allocations,
sync=True,
)
inv = helpers.load_inventory_file(Path(settings.inventory_path))
if not details:
Expand Down
42 changes: 35 additions & 7 deletions manifester/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,15 +171,43 @@ def load_inventory_file(file):
return yaml.load(f, Loader=yaml.FullLoader) or []


def update_inventory(inventory_data):
def _dump_inventory_file(inventory_path, inventory):
"""Write inventory data to local inventory file."""
with inventory_path.open("w") as inventory_file:
yaml.dump(inventory, inventory_file, allow_unicode=True)


def _update_inventory(inventory_path, inventory, allocation):
"""Add new allocation, remove and recreate inventory file."""
inventory.append(allocation)
inventory_path.unlink()
inventory_path.touch()
_dump_inventory_file(inventory_path, inventory)
Comment on lines +174 to +185
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could this be simplified?

Suggested change
def _dump_inventory_file(inventory_path, inventory):
"""Write inventory data to local inventory file."""
with inventory_path.open("w") as inventory_file:
yaml.dump(inventory, inventory_file, allow_unicode=True)
def _update_inventory(inventory_path, inventory, allocation):
"""Add new allocation, remove and recreate inventory file."""
inventory.append(allocation)
inventory_path.unlink()
inventory_path.touch()
_dump_inventory_file(inventory_path, inventory)
def _update_inventory(inventory_path, inventory, allocation):
"""Add new allocation, remove and recreate inventory file."""
inventory.append(allocation)
inventory_path.unlink()
inventory_path.touch()
yaml.dump(inventory, inventory_path, allow_unicode=True)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JacobCallahan Per the PyYAML docs, yaml.dump needs an open file in order to write to it directly. Your proposed changed throws an AttributeError: 'PosixPath' object has no attribute 'write'. Did you mean: 'drive'? when it tries to dump to the Path object. So we need a second line before yaml.dump to open the inventory file. I think it's slightly cleaner to move that action into a separate helper function since it gets used in three places, but that part isn't strictly necessary.



def update_inventory(inventory_data, sync=False, remove=False, uuid=None):
"""Replace the existing inventory file with current subscription allocations."""
inventory_path = Path(settings.inventory_path)
if load_inventory_file(inventory_path):
inventory_path.unlink()
inventory_path.touch()
if inventory_data != []:
with inventory_path.open("w") as inventory_file:
yaml.dump(inventory_data, inventory_file, allow_unicode=True)
if sync:
if load_inventory_file(inventory_path):
inventory_path.unlink()
inventory_path.touch()
if inventory_data != []:
_dump_inventory_file(inventory_path, inventory_data)
elif remove:
inv = load_inventory_file(inventory_path)
_dump_inventory_file(inventory_path, [alloc for alloc in inv if uuid not in alloc["uuid"]])
Copy link
Preview

Copilot AI Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition 'uuid not in alloc["uuid"]' should be 'uuid != alloc["uuid"]' to correctly filter out the allocation with the matching UUID.

Suggested change
_dump_inventory_file(inventory_path, [alloc for alloc in inv if uuid not in alloc["uuid"]])
_dump_inventory_file(inventory_path, [alloc for alloc in inv if uuid != alloc["uuid"]])

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
else:
current_allocation = next(
iter([alloc for alloc in inventory_data if alloc["uuid"] == uuid])
)
inv = load_inventory_file(inventory_path)
uuids = [alloc["uuid"] for alloc in inv]
if current_allocation["uuid"] not in uuids:
_update_inventory(inventory_path, inv, current_allocation)
else:
inv = [alloc for alloc in inv if uuid not in alloc["uuid"]]
_update_inventory(inventory_path, inv, current_allocation)


def fake_http_response_code(good_codes=None, bad_codes=None, fail_rate=0):
Expand Down
13 changes: 7 additions & 6 deletions manifester/manifester.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ def create_subscription_allocation(self):
f"Subscription allocation created with name {self.allocation_name} "
f"and UUID {self.allocation_uuid}"
)
update_inventory(self.subscription_allocations)
update_inventory(self.subscription_allocations, uuid=self.allocation_uuid)
return self.allocation_uuid

def delete_subscription_allocation(self, uuid=None):
Expand All @@ -203,7 +203,9 @@ def delete_subscription_allocation(self, uuid=None):
cmd_args=[f"{self.allocations_url}/{uuid if uuid else self.allocation_uuid}"],
cmd_kwargs=data,
)
update_inventory(self.subscription_allocations)
update_inventory(
self.subscription_allocations, remove=True, uuid=uuid if uuid else self.allocation_uuid
)
return response

def add_entitlements_to_allocation(self, pool_id, entitlement_quantity):
Expand Down Expand Up @@ -322,15 +324,15 @@ def process_subscription_pools(self, subscription_pools, subscription_data):
f"{subscription_data['name']} to the allocation."
)
self._active_pools.append(match)
update_inventory(self.subscription_allocations)
update_inventory(self.subscription_allocations, uuid=self.allocation_uuid)
break
elif add_entitlements.status_code == SUCCESS_CODE:
logger.debug(
f"Successfully added {subscription_data['quantity']} entitlements of "
f"{subscription_data['name']} to the allocation."
)
self._active_pools.append(match)
update_inventory(self.subscription_allocations)
update_inventory(self.subscription_allocations, uuid=self.allocation_uuid)
break
else:
raise RuntimeError(
Expand Down Expand Up @@ -409,7 +411,7 @@ def trigger_manifest_export(self):
manifest.uuid = self.allocation_uuid.uuid
else:
manifest.uuid = self.allocation_uuid
update_inventory(self.subscription_allocations)
update_inventory(self.subscription_allocations, uuid=self.allocation_uuid)
return manifest

def get_manifest(self):
Expand Down Expand Up @@ -437,4 +439,3 @@ def __enter__(self):
def __exit__(self, *tb_args):
"""Deletes subscription allocation on teardown unless using CLI."""
self.delete_subscription_allocation()
update_inventory(self.subscription_allocations)
Loading