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

Machine friendly RPC commands #527

Closed
tomasbrod opened this issue Aug 20, 2017 · 16 comments
Closed

Machine friendly RPC commands #527

tomasbrod opened this issue Aug 20, 2017 · 16 comments
Labels
compatibility deprecation Removed deprecated functionality enhancement

Comments

@tomasbrod
Copy link
Member

I think we should start making the RPC calls more machine friendly. Right now some of them are meant for human reading.
Also the commands intended for general use should be documented.

@Lenni
Copy link

Lenni commented Aug 23, 2017

What should a machine friendly RPC Command look like? I personally think that the commands are fine, even if you call them from other programs.

@denravonska
Copy link
Member

denravonska commented Aug 24, 2017

The JSON data should be translatable to object properties. For example, "magnitude": 4.0 translates to object.magnitude = 4.0 while "Whitelisted Project Count": 24 does not. Or maybe it depends on the language but we shouldn't rely on that. The API is also inconsistent both in naming conventions and overall data structuring.

@TheCharlatan
Copy link
Contributor

#1065 improves the machine friendliness of the polling rpc.

@RoboticMind
Copy link
Contributor

Is this still an issue after the RPC Overhaul?

@denravonska
Copy link
Member

Yeah, the JSON variable names are in many cases written in a human readable form. For example, mymagnitude:

  {
    "CPID": "5a094d7d93f6d6370e78a2ac8c008407",
    "Earliest Payment Time": "08-21-2016 20:50:24",
    "Magnitude (Last Superblock)": 25,
    "Research Payments (14 days)": 97.29000000000002,
    "Daily Paid": 6.949285714285716,
    "Expected Earnings (14 days)": 87.5,
    "Expected Earnings (Daily)": 6.25,
    "Fulfillment %": 111.0109539023277,
    "CPID Lifetime Interest Paid": 1587.077869829997,
    "CPID Lifetime Research Paid": 17295.69999999998,
    "CPID Lifetime Avg Magnitude": 92.30137522820449,
    "CPID Lifetime Payments Per Day": 23.92544494912583,
    "Last Blockhash Paid": "4bc501d41faf7590620d4490e00319b36ed9af8ecc6d887b8846635392c40e0a",
    "Last Block Paid": 1334865,
    "Tx Count": 1583
  },

@iFoggz
Copy link
Member

iFoggz commented Aug 18, 2018

maybe of spacing on the left entry would be more helpful

like "total amount" instead of ("total_amount" and "totalamount")
i see many instances of this practise

@jamescowens
Copy link
Member

We really need to look at this from a big picture perspective and make some decisions to stop the brain damage. I am personally in favor of snake case. We have to decide what to do about all of the legacy keys.... changing older rpc JSON key names could break consumers of that data.

We also ought to be thinking about retrofitting Bitcoin's newer RPC code, which has better argument handling, etc. than ours.

@jamescowens jamescowens added compatibility deprecation Removed deprecated functionality enhancement labels Jun 15, 2020
@jamescowens jamescowens added this to the Gladys milestone Jun 15, 2020
@jamescowens
Copy link
Member

I am going to put this in Gladys for now.

@div72
Copy link
Member

div72 commented Jun 17, 2020

We have to decide what to do about all of the legacy keys.... changing older rpc JSON key names could break consumers of that data.

@cyrossignol @jamescowens Doesn't the RPC changes in #1739 cause this too?

@cyrossignol
Copy link
Member

cyrossignol commented Jun 17, 2020 via email

@jamescowens
Copy link
Member

I think @cyrossignol has the right approach. Maintain maximum compatibility with existing baseline bitcoin functionality, but the Gridcoin rpc commands, many of them have (and are now) redone. I like the snake_case names.

@jamescowens
Copy link
Member

This will not be complete quite frankly until porting over the Bitcoin RPC code. I am going to deassign the milestone on this for the time being. In general we have adopted the standard of using all lower case and underscores for the parameter names in the JSON output for all new rpc commands.

@jamescowens jamescowens removed this from the Hilda milestone Jan 9, 2021
@iFoggz
Copy link
Member

iFoggz commented Jul 7, 2021

still a lot of work to be done on this and I like snake_case however some changes would have to be known at release so anyone using rpc for data gathering would be able to make the changes.

@jamescowens
Copy link
Member

Yes, but remember we are only doing snake_case for Gridcoin specific rpc commands. rpcs that are part of the Bitcoin "api" should follow Bitcoin's standard. there are a few instances where our commands are the same as Bitcoin's but have been extended for Gridcoin functionality. We will have to review those on a case by case basis.

@jamescowens
Copy link
Member

In general, this is a second order problem in my book and there are many other more important things to worry about.

@jamescowens
Copy link
Member

I am closing this issue. We are doing all new rpc commands in snake_case. The breakage on older commands and trying to get everyone to change is not worth the hassle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility deprecation Removed deprecated functionality enhancement
Projects
None yet
Development

No branches or pull requests

9 participants