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

[8.0] FIX Multi VO metadata double VO suffix issue fix (#7697) #7708

Merged

Conversation

martynia
Copy link
Contributor

@martynia martynia commented Jun 27, 2024

BEGINRELEASENOTES

*DMS
FIX: Remove def findDirIDsByMetadata(self, metaDict, dPath, credDict): method from MultiVODirectoryMetadata (derived) class which caused an extra VO suffix added when searching. The method is meant to be used internally only on keys which are already expanded in a MultiVO case. Add a user-level def findDirectoriesByMetadata(self, queryDict, path, credDict) to the derived class thus adding a VO suffix for a directory search. Fixes #7687.

ENDRELEASENOTES

@DIRACGridBot DIRACGridBot added the alsoTargeting:integration Cherry pick this PR to integration after merge label Jun 27, 2024
@martynia martynia changed the title [8.0] FIX Multi VO metadata double VO suffix issu fix (#7697) [8.0] FIX Multi VO metadata double VO suffix issue fix (#7697) Jun 27, 2024
…o query filecatalog.

This is needed for MultiVO*Metadata file catalog configurations
@martynia martynia force-pushed the rel-v8r0_janusz_multiVO_matadataFix branch from 53a982d to 8a90c8e Compare June 27, 2024 15:45
@martynia martynia force-pushed the rel-v8r0_janusz_multiVO_matadataFix branch from f95e2e5 to 27bb1c2 Compare June 27, 2024 17:25
@martynia martynia force-pushed the rel-v8r0_janusz_multiVO_matadataFix branch from 0db8508 to 621ec60 Compare June 28, 2024 06:49
@marianne013
Copy link
Contributor

I'll try to test this. If I haven't by Monday afternoon, please ping me again.

@marianne013
Copy link
Contributor

Simon had the following comment offline, here reproduced for completeness, so I can find it when I have time to get back to this.

[start quote]
This boils down to:

findDirIDsByMetadata is effectively an internal function and should expect
to given a pre-processed meta key name: It's only called by one other
function which has already processed meta name (which is why the double VO
sneaks in). In vanilla DIRAC at least.

I'm not sure the best way to fix that, obviously just removing the
_getMetaNameDict from it works, but if there is some notion of internal and
external functions this should be clear in the function naming or
something. Maybe it's a simple as it should be _findDirIDsByMetadata and
all _ functions shouldn't process the metadata keys?

There may be others that effectively have the same problem?
[end quote]

@martynia
Copy link
Contributor Author

martynia commented Jul 2, 2024

The fix makes findDirIDsByMetadata an internal utility function (removed from the derived class) which does not add or manipulate a VO suffix in any way. In order to maintain a correct directory search findDirectoriesByMetadata(self, queryDict, path, credDict) was added to the derived class which overloads the base class method and adds the suffix directly before calling the base class method. This follows a pattern already present for files.

@marianne013
Copy link
Contributor

I just tested this on dirac00. It now says I am 60% done, which is progress. Now I just need to remember how many files I actually tagged and check that the files are where I think they should be.

@marianne013
Copy link
Contributor

Also, the changes I see in InputDataAgent.py are actually in #7683
Does this need re-basing ? Otherwise we can probably go ahead. One issue at a time.

@andresailer andresailer linked an issue Jul 3, 2024 that may be closed by this pull request
@andresailer andresailer merged commit 96a985c into DIRACGrid:rel-v8r0 Jul 3, 2024
26 checks passed
@DIRACGridBot DIRACGridBot added the sweep:done All sweeping actions have been done for this PR label Jul 3, 2024
DIRACGridBot pushed a commit to DIRACGridBot/DIRAC that referenced this pull request Jul 3, 2024
@DIRACGridBot
Copy link

Sweep summary

Sweep ran in https://github.com/DIRACGrid/DIRAC/actions/runs/9775394680

Successful:

  • integration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alsoTargeting:integration Cherry pick this PR to integration after merge sweep:done All sweeping actions have been done for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MultiVOMetadata doubling VO identifier
4 participants