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
Tickets/DM-30702 Add metadata to QuantumGraph #127
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but check my comments, I think we may want few extra things in metadata.
metadata = {"input": args.input, "output": args.output, "butler_config": args.butler_config, | ||
"output_run": args.output_run, "extend_run": args.extend_run, | ||
"skip_existing": args.skip_existing} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On JIRA @timj also asked about few other items: time of creation, user name, data query (should be in args.data_query
). I am somewhat concerned that butler_config
may point to some temporary location which could disappear, should we be storing registry database URL just in case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the butler URI for the repository itself is probably better than the ButlerConfig
(or at least we should definitely include the butler URI regardless even if we think we also need the config itself).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
args.butler_config
here is just a command line argument which is a path/URL, not the whole ButlerConfig
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. That's not a good name for it then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, historically it comes from --butler-config
command line option which by default was translated to butler_config
attribute name by argparse
(which we don't use any more).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the key in the metadata to say "butler_argument" to better reflect that it was what was passed on the command line, and side step the name butler_config
Record some command line options with the QuantumGraph metadata
02761ba
to
f17ca38
Compare
Checklist
doc/changes