Skip to content
This repository has been archived by the owner on Dec 9, 2024. It is now read-only.

--no-docker_pull prevents looking at remote repositories. #502

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

--no-docker_pull prevents looking at remote repositories. #502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion perfzero/lib/perfzero/perfzero_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,23 @@ def add_setup_parser_arguments(parser):
--build-arg arg0 --build-arg arg1=value1 --build-arg "arg2=value with space" --build-arg arg3=300
'''
)
parser.add_argument(
'--docker_pull',
action='store_true',
dest='docker_pull',
help='''Set to check remote repositories for updated images. This is the
default. Use --no-docker_pull for the opposite.
'''
)
parser.add_argument(
'--no-docker_pull',
Copy link
Member

Choose a reason for hiding this comment

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

I think it's a little confusing to create 2 flags for the same bit.

Can the commandline pass --docker_pull="false" or equivalent to get the same effect?

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you think about this? @sganeshb I think this is much better. I over did it the first time. It was getting late.

That is the downside to argparse you cannot do a boolean like you would with absl. It works fine if you want to only have true and default false. Here is another option that gets a very similar result (maybe the same really) but with less lines.

This creates only a --no-docker_pull and no docker_pull (which makes sense as it is True by default). --no-docker_pull sets docker_pull True by default and then flips to false when --no-docker_pull is used.

Note: that if you pass --docker_pull on the commandline it will fail because it is not a flag, which I think is fine.

  parser.add_argument(
      '--no-docker_pull',
      action='store_false',
      default=True,
      dest='docker_pull',
      help='''Set to skip checking remote repositories for updated images. This
      is useful if the base image has been created locally.
      '''
      )

Copy link
Member

Choose a reason for hiding this comment

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

Thanks - that looks better. Should the default be False to preserve old behavior?

action='store_false',
dest='docker_pull',
help='''Set to skip checking remote repositories for updated images. This
is useful if the base image has been created locally.
'''
)
parser.set_defaults(docker_pull=True)


def add_benchmark_parser_arguments(parser):
Expand Down Expand Up @@ -343,7 +360,7 @@ def get_flags(self):
if value is not None:
not_none_flags[key] = value
return not_none_flags

def get_git_repos(self, site_packages_dir):
"""Parse git repository string."""
git_repos = []
Expand Down
4 changes: 3 additions & 1 deletion perfzero/lib/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ def _create_docker_image(FLAGS, project_dir, workspace_dir,
# dockerfile_path does not exist
dockerfile_path = os.path.join(project_dir, FLAGS.dockerfile_path)
extra_pip_specs = (FLAGS.extra_pip_specs or '').replace(';', '')
docker_base_cmd = 'docker build --no-cache --pull'
docker_base_cmd = 'docker build --no-cache'
if FLAGS.docker_pull:
docker_base_cmd = docker_base_cmd + ' --pull'
# FLAGS.extra_docker_build_args will be a list of strings (e.g. ['a', 'b=c']).
# We treat the strings directly as build-args: --build-arg a --build-arg b=c
# Empty strings are ignored.
Expand Down