-
Notifications
You must be signed in to change notification settings - Fork 648
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
refine es_objects plugin #1271
refine es_objects plugin #1271
Conversation
small update: i was syncronizing the mainnet so i chery picked the commit into the testnet and now the links are working again with testnet data: by the way node started with:
direct GET access to elasticsearch: |
I think some indexes are missing:
|
the fields are there. explain more, thanks. |
Yes the fields are there but are not searchable (cf the little warn signs). |
in some talks with @Zapata we are thinking on 2 things to add in the context of this pull:
|
@oxarbitrage @pmconrad @ryanRfox we need to decide whether we want this in 201810 feature release. |
IMO no. |
I think we should consider having this in the next release, even if still not perfect it is definitely better than the version we currently have available in |
prepare.clear(); | ||
} | ||
} | ||
std::string es_objects_plugin_impl::createIdstring(object_id_type& object_id) | ||
{ | ||
return fc::to_string(object_id.space()) + "." + fc::to_string(object_id.type()) + "." + fc::to_string(object_id.instance()); |
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.
Why not use https://github.com/bitshares/bitshares-core/blob/2.0.180823/libraries/db/include/graphene/db/object_id.hpp#L86 instead? Keep your code DRY.
@@ -47,13 +47,14 @@ class es_objects_plugin_impl | |||
{ curl = curl_easy_init(); } | |||
virtual ~es_objects_plugin_impl(); | |||
|
|||
bool updateDatabase( const vector<object_id_type>& ids , bool isNew); | |||
bool indexDatabase( const vector<object_id_type>& ids, std::string action); | |||
void RemoveFromDatabase( object_id_type id, std::string index); |
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.
Please start method names with a lowercase letter.
I don't mind camelCase, but usually we use underscores instead.
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.
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.
As discussed previously, I haven't done a deep review since this is experimental code.
Looks ok.
Most of the advances in the last time were done in the
elasticsearch
plugin, this pull is to make some changes ates_objects
.In this first commit:
es-objects-keep-only-current
default to true to modify and delete objects.We want to start with the most simple use case and have record of every change complicates a bit the thing. By now this is still possible to do in the plugin changing this option to
false
but i am also considering stuff like https://github.com/ForgeRock/es-change-feed-plugin for object changes.i am currently synchronizing a testnet node with this code, have kibana access at:
http://148.251.10.231:5601/app/kibana#/discover?_g=(refreshInterval:(display:Off,pause:!f,value:0),time:(from:now-5y,mode:quick,to:now))&_a=(columns:!(_source),index:%27917a6c90-a1a7-11e8-b559-4b33a8fc253e%27,interval:auto,query:(language:lucene,query:%27_index:objects-asset%27),sort:!(block_time,desc))
and direct GET access to ES at: http://148.251.10.231:5005/
We should focus initially in this 3 indexes:
objects-asset
objects-account
object-balance
We should be able to get proxy data easy, get referer data easy, get all markets(by crossing all assets), get holder data(making use of balance object - can replace node asset_api).