steemit / steem-python

The official Python (3) library for the Steem Blockchain.
https://steem.io
MIT License
154 stars 100 forks source link

SQL Config Discussion #216

Closed cyon1c closed 6 years ago

cyon1c commented 6 years ago

Discuss changes for SQL Config to generate spec for #215 .

roadscape commented 6 years ago

Relevant https://github.com/steemit/steem-python/issues/75 Disk access should not be required to make http requests https://github.com/steemit/steem-python/issues/77 steemd nodes configuration should not be stored in sqlite database https://github.com/steemit/steem-python/compare/fix-install-issues

cyon1c commented 6 years ago

References to configStorage in the codebase:

steem/instance.py:2
steem/commit.py:23
steem/cli.py:33
steem/dex.py:1
steem/steemd.py:1
steem/wallet.py:6
cyon1c commented 6 years ago

Primary functions for the sql config are:

  1. storing a default account + credentials
  2. Setting some (presumably) sane defaults:
    config_defaults = {
        "categories_sorting": "trending",
        "default_vote_weight": 100.0,
        "format": "markdown",
        "limit": 10,
        "list_sorting": "trending",
        "post_category": "steem",
        "prefix": "STM"
    }
  3. Storing a list of nodes.
roadscape commented 6 years ago

Does it store just 1 account/creds or does it have multi account support?

cyon1c commented 6 years ago

Multi-account. Relevant info in Wallet.py class documentation at top.

cyon1c commented 6 years ago

I propose we leave the account and defaults functionality in place, however remove any references to nodes from the storage and simply offer a default of https://api.steemit.com if none are specified by the user. And properly handle taking a list or single node.

for reference, internally we hardly use the defaults: In a grep of steem/, the only defaults (besides accounts) referenced are:

default_vote_weight:2x prefix:1x

roadscape commented 6 years ago

Some thoughts, from the perspective that steem-python (particularly core) functions need to be seriously simplified and made more reliable if a wallet is ever going to be built on it:

  1. Too much sugar, we need a solid and stable library; keys like "categories_sorting": "trending", and "default_vote_weight": 100.0, seem extraneous.
  2. File-based config must be optional; one should be able to use steempy without writing to disk
  3. How API nodes are handled is obtuse; agree it should be more explicit. If we're removing this from sqlite we should probably first block writing to it, but continue to read (while printing deprecation messages), so existing scripts are not interrupted.
  4. A specific storage should not be assumed.. there should be a single class which handles reading/writing config to sqlite (thus easily switched to e.g. json). If this is not the case we need it.
cyon1c commented 6 years ago

1 - Are you thinking that all of the config defaults should be pulled as too much sugar, or just those keys in particular? 2 - I like the thought of approaching configs through command line arguments and env vars in a .conf like file. 3 - This seems a reasonable enough approach short term. In the long term, would we want nodes to be specified through the same approach as the config values? 4 - Agreed. Functionality should be storage type agnostic.

cyon1c commented 6 years ago

220 and #221 are issues created in response to this issue. I think they are sufficient based on discussion from this issue to close it.