Tools: update release date checker to check for release date vs. last commit of release#16712
Tools: update release date checker to check for release date vs. last commit of release#16712mattklein123 merged 8 commits intoenvoyproxy:mainfrom moderation:main
Conversation
…vs. last commit date. Signed-off-by: Michael Payne <michael@sooper.org>
repokitteh-read-only
bot
added
the
deps
Signed-off-by: Michael Payne <michael@sooper.org>
Signed-off-by: Michael Payne <michael@sooper.org>
phlax
left a comment
phlax
left a comment
There was a problem hiding this comment.
hi @moderation - it would have been helpful to build on the previous pr rather than opening a new one
i have flagged a few nits
tools/dependency/release_dates.py
Outdated
| except github.GithubException as e: | ||
| print('GithubException {e} while getting latest release.') | ||
| latest = '' | ||
| pass |
There was a problem hiding this comment.
this pass doesnt do anything
im wondering if it should return here - ie if its thrown a GithubException im thinking repo.get_tags will also fail
There was a problem hiding this comment.
I've improved the error handling using the Github exception data and added comments explaining why it is OK to not find latest releases in some cases and keep processing.
There was a problem hiding this comment.
ok - but the pass is still superfluous
Signed-off-by: Michael Payne <michael@sooper.org>
Signed-off-by: Michael Payne <michael@sooper.org>
Signed-off-by: Michael Payne <michael@sooper.org>
| mismatch = '' | ||
| iso_release_date = format_utc_date(github_release_date) | ||
| print(f'{dep} has a GitHub release date {iso_release_date}') | ||
| print(f'{Fore.GREEN}{dep} has a GitHub release date {iso_release_date}{Style.RESET_ALL}') |
There was a problem hiding this comment.
i would move this after the condition/raise - and in the raise provide more info about the mismatch
There was a problem hiding this comment.
The way this is structured today makes finding the date mismatches easy to find. The script outputs the real GH date followed by the exception that shows the erroneous date. I don't expect many people to view this but helpful for myself and the deps maintainers. But let me know if I'm missing something
There was a problem hiding this comment.
kinda unreliable iiuc - because one is going to stdout and the other to stderr
there is no guarantee that both will be shown and if they are that they will be in correct order
not a blocker - but that is the reason i suggested this
There was a problem hiding this comment.
(also that it flashes green even when its about to fail)
tools/dependency/release_dates.py
Outdated
| except github.GithubException as e: | ||
| print('GithubException {e} while getting latest release.') | ||
| latest = '' | ||
| pass |
There was a problem hiding this comment.
ok - but the pass is still superfluous
Signed-off-by: Michael Payne <michael@sooper.org>
Signed-off-by: Michael Payne <michael@sooper.org>
phlax
left a comment
phlax
left a comment
There was a problem hiding this comment.
lgtm, thanks @moderation
as discussed offline - this needs some further updates in terms of moving to bazel and adding tests - but in terms of the scope of this PR i think its gtm
… commit of release (envoyproxy#16712) Signed-off-by: Michael Payne <michael@sooper.org>
3 participants
Tools: update release date checker to check for release date vs. last commit of release
Additional Description:
sixfrom pythonhosted to Github location so we can verify release datebazel/repository_locations.bzlandapi/bazel/repository_locations.bzlso thatrelease_datefollowsversionandsha256to aid with dependency maintainancetools/dependency/release_dates.pyto check for release metadata vs. current approach of checking for latest commit in a release which may not equal release date. /cc @htuch + @phlax for Python review.Risk Level: Low
Testing:
bazel --nohome_rc test //test/...,bazel --nohome_rc test @envoy_api_canonical//test/... @envoy_api_canonical//tools/...,bazel --nohome_rc build @envoy_api_canonical//envoy/...,tools/dependency/release_dates.py bazel/repository_locations.bzlDocs Changes: None required
Release Notes: None required
Signed-off-by: Michael Payne michael@sooper.org