[BE] Remove deprecated github gql param and disable inconsistent test#107385
[BE] Remove deprecated github gql param and disable inconsistent test#107385ZainRizvi wants to merge 1 commit intogh/ZainRizvi/7/basefrom
Conversation
[ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/107385
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit d438bd9 with merge base aa9f6a4 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
| print( | ||
| f"Can't get commit {pr.last_commit()['oid']} pushed date. Is it merge commit by chance?" | ||
| ) | ||
| elif (datetime.utcnow() - cast(datetime, pr.last_pushed_at())).days > stale_pr_days: |
There was a problem hiding this comment.
is there an equivalent for pusheddate that we can use? or are we fine with getting rid of the 3 day check for old jobs?
There was a problem hiding this comment.
The deprecation notice doesn't suggest any alternatives, and a cursory glance at the API didn't suggest anything either.
Frankly, this felt like a weird check that didn't really help with anything. The team seems to have originally thought this was a merge-base date check instead of a "last push" date check, the latter of which doesn't really offer much protection.
I do think we should add in a merge-base date check, but that's a separate project from getting rid of this deprecated code. For the past month this code has been doing nothing anyways (I only discovered the failure because the test started failing once I regenerated the github gql query cache. Hence this PR being in a ghstack)
This is a strict improvement over the existing code.
Fixes #100406 Pull Request resolved: #107390 Approved by: https://github.com/clee2000 ghstack dependencies: #107385
Stack from ghstack (oldest at bottom):
Two fixes:
pushDate, which has been deprecated and now always returns nulltest_merge_ghstack_intowhich was recently added in [GHF][mergebot] record ghstack dependencies in the commit message #105251. This test used the results of another person's ghstack PR for testing, but as the dev submitted chunks of their PR this test's assumptions have been broken. cc @izaitsevfb for a long term fix here