Skip to content

Commit

Permalink
Merge pull request #18631 from jmchilton/remove_dataset_create
Browse files Browse the repository at this point in the history
Remove unused functions in dataset managers
  • Loading branch information
jmchilton authored Aug 3, 2024
2 parents d7496f4 + 0c8666f commit c3195cf
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 28 deletions.
17 changes: 0 additions & 17 deletions lib/galaxy/managers/datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,23 +64,6 @@ def __init__(self, app: MinimalManagerApp):
self.quota_agent = app.quota_agent
self.security_agent = app.model.security_agent

def create(self, manage_roles=None, access_roles=None, flush=True, **kwargs):
"""
Create and return a new Dataset object.
"""
# default to NEW state on new datasets
kwargs.update(dict(state=(kwargs.get("state", model.Dataset.states.NEW))))
dataset = model.Dataset(**kwargs)
self.session().add(dataset)

self.permissions.set(dataset, manage_roles, access_roles, flush=False)

if flush:
session = self.session()
with transaction(session):
session.commit()
return dataset

def copy(self, item, **kwargs):
raise exceptions.NotImplemented("Datasets cannot be copied")

Expand Down
6 changes: 0 additions & 6 deletions lib/galaxy/managers/histories.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,6 @@ def p_tag_filter(term_text: str, quoted: bool):
stmt = stmt.offset(payload.offset)
return trans.sa_session.scalars(stmt), total_matches # type:ignore[return-value]

def copy(self, history, user, **kwargs):
"""
Copy and return the given `history`.
"""
return history.copy(target_user=user, **kwargs)

# .... sharable
# overriding to handle anonymous users' current histories in both cases
def by_user(
Expand Down
13 changes: 10 additions & 3 deletions test/unit/app/managers/test_DatasetManager.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ def set_up_managers(self):
super().set_up_managers()
self.dataset_manager = DatasetManager(self.app)

def create_dataset_with_permissions(self, manage_roles=None, access_roles=None) -> model.Dataset:
dataset = self.dataset_manager.create(flush=False)
self.dataset_manager.permissions.set(dataset, manage_roles, access_roles)
return dataset

def test_create(self):
self.log("should be able to create a new Dataset")
dataset1 = self.dataset_manager.create()
Expand Down Expand Up @@ -125,7 +130,7 @@ def test_create_public_dataset(self):
)
owner = self.user_manager.create(**user2_data)
owner_private_role = self.user_manager.private_role(owner)
dataset = self.dataset_manager.create(manage_roles=[owner_private_role])
dataset = self.create_dataset_with_permissions(manage_roles=[owner_private_role])

permissions = self.dataset_manager.permissions.get(dataset)
assert isinstance(permissions, tuple)
Expand Down Expand Up @@ -157,7 +162,9 @@ def test_create_private_dataset(self):
self.log("should be able to create a new Dataset and give it private permissions")
owner = self.user_manager.create(**user2_data)
owner_private_role = self.user_manager.private_role(owner)
dataset = self.dataset_manager.create(manage_roles=[owner_private_role], access_roles=[owner_private_role])
dataset = self.create_dataset_with_permissions(
manage_roles=[owner_private_role], access_roles=[owner_private_role]
)

permissions = self.dataset_manager.permissions.get(dataset)
assert isinstance(permissions, tuple)
Expand Down Expand Up @@ -268,7 +275,7 @@ def test_serialize_permissions(self):

def test_serializers(self):
# self.user_manager.create( **user2_data )
dataset = self.dataset_manager.create()
dataset = self.dataset_manager.create(state=model.Dataset.states.NEW)
all_keys = list(self.dataset_serializer.serializable_keyset)
serialized = self.dataset_serializer.serialize(dataset, all_keys)

Expand Down
4 changes: 2 additions & 2 deletions test/unit/app/managers/test_HistoryManager.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def test_base(self):
== self.trans.sa_session.execute(select(model.History).filter(model.History.user == user2)).scalar_one()
)

history2 = self.history_manager.copy(history1, user=user3)
history2 = history1.copy(target_user=user3)

self.log("should be able to query")
histories = self.trans.sa_session.scalars(select(model.History)).all()
Expand Down Expand Up @@ -108,7 +108,7 @@ def test_copy(self):
self.app.tag_handler.set_tags_from_list(user=user2, item=hda, new_tags_list=hda_tags)
self.hda_manager.annotate(hda, hda_annotation, user=user2)

history2 = self.history_manager.copy(history1, user=user3)
history2 = history1.copy(target_user=user3)
assert isinstance(history2, model.History)
assert history2.user == user3
assert history2 == self.trans.sa_session.get(model.History, history2.id)
Expand Down

0 comments on commit c3195cf

Please sign in to comment.