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

Allow commit range detection to fail by falling back to rebuilding images #90

Open
consideRatio opened this issue Aug 25, 2020 · 0 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@consideRatio
Copy link
Collaborator

consideRatio commented Aug 25, 2020

While it is a bad practice to use git push --force, it would be good to get hubploy's commit range detection to not crash hard if it used. Currently, the commit range detection running within a GitHub Actions environment fail like below, and can be inspected in this github action job.

fatal: Invalid symmetric difference expression fa8d85b09fd6605d7efa95bb0d52e3a5e9a1f871...HEAD
Building 1 images
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.7.8/x64/bin/hubploy", line 11, in <module>
    load_entry_point('hubploy==0.1.1', 'console_scripts', 'hubploy')()
  File "/opt/hostedtoolcache/Python/3.7.8/x64/lib/python3.7/site-packages/hubploy/__main__.py", line 120, in main
    if image.needs_building(check_registry=args.check_registry, commit_range=args.commit_range):
  File "/opt/hostedtoolcache/Python/3.7.8/x64/lib/python3.7/site-packages/hubploy/config.py", line 148, in needs_building
    return utils.path_touched(self.path, commit_range=commit_range)
  File "/opt/hostedtoolcache/Python/3.7.8/x64/lib/python3.7/site-packages/hubploy/utils.py", line 68, in path_touched
    'git', 'diff', '--name-only', commit_range, '--', *paths
  File "/opt/hostedtoolcache/Python/3.7.8/x64/lib/python3.7/subprocess.py", line 411, in check_output
    **kwargs).stdout
  File "/opt/hostedtoolcache/Python/3.7.8/x64/lib/python3.7/subprocess.py", line 512, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['git', 'diff', '--name-only', 'fa8d85b09fd6605d7efa95bb0d52e3a5e9a1f871...HEAD', '--', '/home/runner/work/nh2020-jupyterhub/nh2020-jupyterhub/deployments/hub-***-org/image']' returned non-zero exit status 128.
##[error]Process completed with exit code 1.

The issue is that the checked out git repo has the latest git state, but not the --force-fully overridden state. Hubploy will use the GitHub Action system provided information of the latest git hash, but then assume that it is available and crash when it isn't.

https://github.com/yuvipanda/hubploy/blob/8a55ab078d6b25afcf292846905c3107ec83f9a8/hubploy/commitrange.py#L29-L39

Idea 1

I think the sensible action point is to figure out a fallback when this happens.

Idea 1 conclusion

I concluded that the hubploy fallback in main is to fail if automatic detection fails and no --commit-range was detected.

https://github.com/yuvipanda/hubploy/blob/8a55ab078d6b25afcf292846905c3107ec83f9a8/hubploy/__main__.py#L97-L105

Which means that the approach to return None from the get_commit_range -> get_commit_range_github when the commit is invalid will only lead to the sys.exit(1) call with a note of needing to manually specify the commit range.

https://github.com/yuvipanda/hubploy/blob/8a55ab078d6b25afcf292846905c3107ec83f9a8/hubploy/commitrange.py#L11-L17

Idea 2

  1. We make a check if the commit exists, and let the get_commit_range_github function return None and print a warning that it did so due to a non-existing commit.
  2. We allow for a low-performance fallback when the --commit-range flag isn't set and the automatic commit range detection fail.

Question 1

Does it make sense to do (2) in Idea 2 above?

Answer 1

Yes! It seems like the commit-range is only used to decide if the image needs rebuilding.

https://github.com/yuvipanda/hubploy/blob/8a55ab078d6b25afcf292846905c3107ec83f9a8/hubploy/__main__.py#L118-L120

https://github.com/yuvipanda/hubploy/blob/8a55ab078d6b25afcf292846905c3107ec83f9a8/hubploy/config.py#L135-L148

Conclusion 2

#91 is meant to resolve Idea 2's first part, and it should make sense to just declare that the image needs to be rebuilt if a force push has been made I think.

@consideRatio consideRatio changed the title GitHub actions commit range detection fail hard if git push --force is used Allow commit range detection to fail by falling back to rebuilding images Aug 25, 2020
@consideRatio consideRatio added bug Something isn't working enhancement New feature or request labels Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant