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

[24.0] Fix History Dataset Association creation so that hid is always set #18036

Merged
merged 12 commits into from
Apr 23, 2024
Merged

[24.0] Fix History Dataset Association creation so that hid is always set #18036

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b1333e0
Also perform strict checks for new database objects
mvdbeek Apr 21, 2024
1b1094b
Set (copied) output hid before queuing job
mvdbeek Apr 21, 2024
e3aba2d
Relax requirement for DCEs of collections that are not populated
mvdbeek Apr 21, 2024
5648478
Adjust schema for collection elements of collections that aren't popu…
mvdbeek Apr 21, 2024
bf34997
Fix unitialize typo
mvdbeek Apr 21, 2024
5aecc08
Fall back to populated method if collection not committed yet
mvdbeek Apr 21, 2024
d889002
Commit only after copying library item to history
mvdbeek Apr 21, 2024
e1a45a2
Drop unused copy_from_ldda method
mvdbeek Apr 21, 2024
a56f642
Drop unused method
mvdbeek Apr 21, 2024
badbd3a
Fix premature flush in hda_manager.copy
mvdbeek Apr 22, 2024
c2140bd
Avoid flush in set_meta by using MetadataFile.uuid
mvdbeek Apr 22, 2024
121f27e
Default HID to -1 if for whatever reason we didn't record a HID
mvdbeek Apr 22, 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
32 changes: 23 additions & 9 deletions lib/galaxy/model/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,18 +174,32 @@ def versioned_objects_strict(iter):
yield obj


if os.environ.get("GALAXY_TEST_RAISE_EXCEPTION_ON_HISTORYLESS_HDA"):
log.debug("Using strict flush checks")
versioned_objects = versioned_objects_strict # noqa: F811
def get_before_flush_handler():
if os.environ.get("GALAXY_TEST_RAISE_EXCEPTION_ON_HISTORYLESS_HDA"):
log.debug("Using strict flush checks")

def before_flush(session, flush_context, instances):
for obj in session.new:
Copy link
Member

Choose a reason for hiding this comment

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

Isn't that what versioned_objects_strict() does? If so, why not do something like this:

for obj in versioned_objects_strict(session.new):
    pass

Copy link
Member Author

@mvdbeek mvdbeek Apr 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It creates a version, which doesn't make sense for new objects.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yes, I could do that, though it's not related to versioning objects, which is what threw my off in your comment.

Copy link
Member

Choose a reason for hiding this comment

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

And then, if I'm not mistaken, you wouldn't need this if/then that checks os.environ and contains 2 similar function definitions, and just use the existing logic, I think? i.e., setting versioned_objects = versioned_objects_strict would still work?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think one is better than the other with the exception this one is slightly better since we don't run the os.environ check in the module space ?

if hasattr(obj, "__strict_check_before_flush__"):
obj.__strict_check_before_flush__()
for obj in versioned_objects_strict(session.dirty):
obj.__create_version__(session)
for obj in versioned_objects_strict(session.deleted):
obj.__create_version__(session, deleted=True)

else:

def before_flush(session, flush_context, instances):
for obj in versioned_objects(session.dirty):
obj.__create_version__(session)
for obj in versioned_objects(session.deleted):
obj.__create_version__(session, deleted=True)

return before_flush


def versioned_session(session):
@event.listens_for(session, "before_flush")
def before_flush(session, flush_context, instances):
for obj in versioned_objects(session.dirty):
obj.__create_version__(session)
for obj in versioned_objects(session.deleted):
obj.__create_version__(session, deleted=True)
event.listens_for(session, "before_flush")(get_before_flush_handler())


def ensure_object_added_to_session(object_to_add, *, object_in_session=None, session=None) -> bool:
Expand Down