Skip to content

Parallel work packages processing#48

Merged
wonder-sk merged 6 commits intomainfrom
parallel_work_packages_processing
Feb 14, 2023
Merged

Parallel work packages processing#48
wonder-sk merged 6 commits intomainfrom
parallel_work_packages_processing

Conversation

@ldebek
Copy link
Contributor

@ldebek ldebek commented Feb 14, 2023

  • download/update projects in parallel
  • push projects in parallel
  • set parallelism with --max-workers CLI argument (by default 8)
  • retry push when it fails - e.g. network issue (up to 3 times)
  • do not fail if removal of temp dir fails (sometimes happens on windows)

parser = argparse.ArgumentParser()
parser.add_argument("mergin_project", nargs="?")
parser.add_argument("--cache-dir", nargs="?")
parser.add_argument("--max_workers", nargs="?", type=int, const=8, default=8)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
parser.add_argument("--max_workers", nargs="?", type=int, const=8, default=8)
parser.add_argument("--max-workers", type=int, default=8)

not sure if we need the "nargs" and "const" specified? also better to use dash --max-workers than underscore.

I guess the --cache-dir and the positional mergin_project arguments above probably also do not need to use nargs="?") - a likely extra addition for me when I did not know much about argparse :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nargs="?" is to specify argument which can be optional and I would keep it. "const=8" and "default=8" are to handle all possible cases like:

  • no flag at all
  • flag without value specified
  • flag with value specified

wp_new = set()

for wp in wp_config.wp_names:
def prepare_work_packages(wp):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def prepare_work_packages(wp):
def prepare_work_package(wp):

(it is just one that we prepare inside)

mergin.client_push.push_project_wait(job)
mergin.client_push.push_project_finalize(job)
return True
def push_mergin_project(mc, directory, max_retires=3, sleep_time=5):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def push_mergin_project(mc, directory, max_retires=3, sleep_time=5):
def push_mergin_project(mc, directory, max_retries=3, sleep_time=5):

"""Push data to all Mergin Maps projects"""

for wp in wp_config.wp_names:
def push_work_packages(wp):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def push_work_packages(wp):
def push_work_package(wp):

if push_mergin_project(ctx.mc, wp_dir):
print("Uploaded a new version: " + mergin.MerginProject(wp_dir).metadata["version"])
else:
print("No changes (not creating a new version).")
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the indentation went wrong here, and all this code would be run only when if wp_name in wp_new

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.

2 participants