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

Metadata API: Consider including filename in TargetFile/MetaFile #1411

Closed
jku opened this issue May 21, 2021 · 6 comments
Closed

Metadata API: Consider including filename in TargetFile/MetaFile #1411

jku opened this issue May 21, 2021 · 6 comments
Assignees
Labels
backlog Issues to address with priority for current development goals

Comments

@jku
Copy link
Member

jku commented May 21, 2021

Currently TargetFile and Metafile do not contain the filename of the file they represent. The API itself does not need it but it could be useful for users of the api.

As an example the current client returns a dict for get_one_valid_targetinfo(): that dict contains a filepath field and and a targetinfo field (essentially TargetFile). I would like to keep a similar API but avoid hand-crafted dicts. It would be much nicer to return a TargetFile that would contain the full "metadata" of the targetfile.

I'm not 100% sure if there are similar advantages to MetaFile

@jku
Copy link
Member Author

jku commented May 21, 2021

relates to #1317

@MVrachev MVrachev added the backlog Issues to address with priority for current development goals label May 25, 2021
@sechkova sechkova mentioned this issue Jul 29, 2021
3 tasks
@MVrachev
Copy link
Collaborator

MVrachev commented Aug 2, 2021

I think it makes sense to implement this for the use case that we are sure about - add a path in TargetFile and then
if one day we see a benefit for adding the name in MetaFile then we should add it.
Do you agree @jku?
I can propose a pr for this.

@MVrachev
Copy link
Collaborator

MVrachev commented Aug 3, 2021

I am working on this one already.

@MVrachev
Copy link
Collaborator

MVrachev commented Aug 3, 2021

I decided to propose a pr adding target name in TargetFile, so that the use case we are sure about is addressed and after that, we can discuss if we want to do the same thing for MetaFile.

@MVrachev
Copy link
Collaborator

I had a look in our code at how we are using MetaFile and I can't find a place where adding filename to the MetaFile class will make the code cleaner or easier to understand.
I think we can close this issue for now and if we see the need in the future, then we can reopen this issue or open another one.
What do you think @jku?

@jku
Copy link
Member Author

jku commented Aug 31, 2021

Sounds good.

@jku jku closed this as completed Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issues to address with priority for current development goals
Projects
None yet
Development

No branches or pull requests

2 participants