ibm-quantum / Hub-Automation

Apache License 2.0
9 stars 5 forks source link

Add backend system info retrieval script #7

Closed pshires closed 3 years ago

pshires commented 3 years ago

closes #2

@kwehden @Matt-Stypulkoski Let me know what you think. Thank you.

Matt-Stypulkoski commented 3 years ago

I think overall everything looks good. If the main data you are interested in is the full backend object's data, then I think it is fine. But if the backend names are the important piece of data, then it might be ok to switch that to be the default, and have the rest of the information be given if the specific parameter is given. What do you think @pshires?

pshires commented 3 years ago

That's a good thought. The main goal for me is to have the backend names, so making that the default behavior seems fine to me. I'll go ahead and make printing the backend names the default and have an optional flag to specify the printing of the rest of the information.

However, using print statements in general does make me wonder about the intended usage of this client. I think that due to this client being mostly standalone python scripts, having print statements is good and even desirable. But in the context of a larger application, a print statement would likely be irrelevant. I'm thinking that ultimately that for my use case I will need to develop a library style API client, to be used within some of our larger applications (probably a Ruby Gem to be specific)

Matt-Stypulkoski commented 3 years ago

However, using print statements in general does make me wonder about the intended usage of this client. I think that due to this client being mostly standalone python scripts, having print statements is good and even desirable. But in the context of a larger application, a print statement would likely be irrelevant. I'm thinking that ultimately that for my use case I will need to develop a library style API client, to be used within some of our larger applications (probably a Ruby Gem to be specific)

That is a good point. At the moment, the scripts in this repo are fairly basic, and not set up in any full library format. Most of them perform a single action, or return data. I think that having the script print out this information is fine, as even just seeing that information in the terminal can be useful. It would also give users insight into how to implement this endpoint into a larger script/application that can utilize the returned data.

My initial thought for this repo is to provide users with the tools to perform these basic tasks through small scripts. Though, ultimately to get full use out of these endpoints users will need to, as you said, create a more in depth application/library on their own side.

Matt-Stypulkoski commented 3 years ago

But the script looks good, and everything seems to work as intended. I will merge this into the main branch. Thanks for contributing! And if you have any questions of course do not hesitate to ask!

pshires commented 3 years ago

Awesome, thanks for reviewing!