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

scripts start observers #178

Merged
merged 16 commits into from
Oct 27, 2022
Merged

Conversation

miiu96
Copy link
Contributor

@miiu96 miiu96 commented Oct 6, 2022

  • Added scripts that will start 4 observers and 4 indexer binaries that can index data from testnet, devnet or mainnet based on the .env config file
  • Refactor and improve how the received data is processed (altered accounts are no longer extracted on the indexer side)


try:
shutil.rmtree(working_dir)
print("done")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

print(f"removed directory: {working_dir}")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

shutil.rmtree(working_dir)
print("done")
except:
print('did nothing')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"nothing to clean" / "directory not removed" or simply pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored

update_toml_node(node_config, shard_id)
update_toml_indexer(indexer_config, shard_id)


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added main for all files

data1 = toml.load(path+"/prefs.toml")
data1['config']['web-socket']['server-url'] = str(shard_id)
if shard_id != METACHAIN:
data1['config']['web-socket']['server-url'] = "localhost:" + str(22111+shard_id)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is 22111 the port base? Perhaps define a constant or receive it in the env file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a constant


def update_toml_indexer(path, shard_id):
# prefs.toml
data1 = toml.load(path+"/prefs.toml")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit of renaming: data1 vs. prefs_data or something similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed


def update_toml_indexer(path, shard_id):
# prefs.toml
data1 = toml.load(path+"/prefs.toml")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If path is a pathlib.Path, then you can also use the / operator. For example: https://github.com/ElrondNetwork/elrond-sdk-images/blob/main/scripts/build_contract_rust_within_docker.py#L104

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored.

import sys
import shutil
from pathlib import Path
from git import Repo

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem from the standard library. Perhaps add a requirements.txt file and a README explanation?

For example:

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

# build binary indexer
print("building indexer...")
os.chdir("../../cmd/elasticindexer")
os.system("go build")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually, we favor subprocess instead of os.system. Example:

(rosetta-ci is a private repository)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


# prepare observers
print("preparing config...")
prepare_observer(0)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

os.chdir(working_dir_observer + "/node")
command = "./node" + " --log-level *:DEBUG --no-key --log-save"
# os.system("gnome-terminal 'bash -c \"" + command + ";bash\"'")
os.system("screen -d -m -S obs" + str(shard_id) + " " + command)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see, we're using screen. Perhaps get a second opinion from @valentin-lup, if this is all right.

@@ -0,0 +1,4 @@
import os

os.system("killall screen")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will kill all screen instances that the user has. Perhaps we should find another solution for this, or remove the stop.py script in order to avoid undesired effects?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

@miiu96 miiu96 self-assigned this Oct 27, 2022
@miiu96 miiu96 marked this pull request as ready for review October 27, 2022 07:10
updatePropNFT *data.NFTDataUpdate
tokenInfo *data.TokenInfo
delegator *data.Delegator
processed bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move processed bool to the last position

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

ELROND_GO_BRANCH="feat/altered-accounts"

WORKING_DIRECTORY="IndexerObservers"
OBSERVER_DIR="observer_shard_"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might rename to OBSERVER_DIR_PREFIX

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

working_dir = Path(Path.home() / os.getenv('WORKING_DIRECTORY'))

try:
shutil.rmtree(working_dir)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use a default path for the scripts, not the user's home

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored

@@ -80,7 +80,11 @@ def prepare_observer(shard_id, working_dir, config_folder):

def main():
load_dotenv()
working_dir = Path(Path.home() / os.getenv('WORKING_DIRECTORY'))
working_dir_var = os.getenv('WORKING_DIRECTORY')
if working_dir_var == "":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicated code. might extract a function an use it in the both places

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

bogdan-rosianu
bogdan-rosianu previously approved these changes Oct 27, 2022
bogdan-rosianu
bogdan-rosianu previously approved these changes Oct 27, 2022
andreibancioiu
andreibancioiu previously approved these changes Oct 27, 2022

current_directory = os.getcwd()
# start observer
os.chdir(working_dir_observer + "/node")
os.chdir(Path(working_dir_observer / "node"))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cast to Path is not required (here and below).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

from dotenv import load_dotenv
from utils import *

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually, we should be importing modules without using the wildcard *.


# build binary elrond-go
print("building node...")
subprocess.check_call(["go", "build"], cwd=Path(elrond_go_folder / "cmd/node"))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@miiu96 miiu96 dismissed stale reviews from andreibancioiu and bogdan-rosianu via 3fed07b October 27, 2022 13:42
@miiu96 miiu96 merged commit dc2a42c into feat/altered-accounts Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants