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.

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

Patch 6348 list rules keyerror #6377

Merged
merged 1 commit into from Mar 15, 2024

Conversation

voetberg
Copy link
Contributor

Addressing issue #6348 and adding a test for it.
I missed the commit message format in the contrib guidelines first time around - so we have some duplicate commits from the rebase.

@voetberg voetberg added the bug label Nov 16, 2023
Copy link
Contributor

@dchristidis dchristidis left a comment

Choose a reason for hiding this comment

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

Welcome! I have impression that you misunderstood the issue. No changes are expected in the Rucio client. Instead, you should modify the core to include the missing information.

@stream_session
def list_associated_rules_for_file(scope, name, *, session: "Session"):
"""
List replication rules a file is affected from.
:param scope: Scope of the file.
:param name: Name of the file.
:param session: The database session in use.
:raises: RucioException
"""
rucio.core.did.get_did(scope=scope, name=name, session=session) # Check if the did acually exists
query = session.query(models.ReplicationRule).\
with_hint(models.ReplicaLock, "INDEX(LOCKS LOCKS_PK)", 'oracle').\
join(models.ReplicaLock, models.ReplicationRule.id == models.ReplicaLock.rule_id).\
filter(models.ReplicaLock.scope == scope, models.ReplicaLock.name == name).distinct()
try:
for rule in query.yield_per(5):
yield rule.to_dict()
except StatementError as exc:
raise RucioException('Badly formatted input (IDs?)') from exc

Copy link
Contributor

@dchristidis dchristidis left a comment

Choose a reason for hiding this comment

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

Forgive me for neglecting this for so long.

Please try to mimic #5978 #6297: use this opportunity to make improvements to the existing code (SQLAlchemy 2.0 syntax), but as a separate commit. Refer to the discussion in that PR if curious for the justification.

lib/rucio/core/rule.py Outdated Show resolved Hide resolved
lib/rucio/core/rule.py Outdated Show resolved Hide resolved
@voetberg voetberg force-pushed the patch-6348-list_rules_keyerror branch from 6f66c1d to bdecfac Compare February 19, 2024 20:42
lib/rucio/core/rule.py Outdated Show resolved Hide resolved
@dchristidis
Copy link
Contributor

We’ve had to give priority to the other PR. Now that it is merged, kindly amend your work over the latest master branch. Thank you in advance.

@voetberg voetberg force-pushed the patch-6348-list_rules_keyerror branch 2 times, most recently from f7088ef to 2dd8168 Compare March 15, 2024 13:02
lib/rucio/core/rule.py Outdated Show resolved Hide resolved
@voetberg voetberg force-pushed the patch-6348-list_rules_keyerror branch from 2dd8168 to d44b665 Compare March 15, 2024 13:18
Copy link
Contributor

@dchristidis dchristidis left a comment

Choose a reason for hiding this comment

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

Marvelous. Many thanks and sorry about the long delay. @bari12 I propose to make a freeze exception.

@bari12 bari12 merged commit 145fac0 into rucio:master Mar 15, 2024
27 of 28 checks passed
@voetberg voetberg deleted the patch-6348-list_rules_keyerror branch April 23, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants