- User Since
- Aug 22 2016, 4:36 PM (26 w, 3 d)
Fri, Feb 17
Wed, Feb 15
Fri, Feb 10
learn to count parens
In exchange, here is a cat on a mat trying to do some yoga. Not my cat, but she was at an airbnb I stayed at and my decision to not steal her came down to a coin toss
that's a great pup. 10/10
Thu, Feb 9
Wed, Feb 8
Yeah I think it probably makes sense to not distinguish between a permission error and invalid resource for the reasons you said. I know amazon S3 does a similar thing and just returns a 403 in both cases.
Update message to account for permission error
I just ran into this again but for a completely different reason. The user assured me multiple times that they were entering a valid diff ID and I repeatedly explained, "no, you don't understand. I Know What I'm Doing™".
Just to clarify, I tested it locally, not on this install.
Tue, Feb 7
Just spent some time tracking down what ended up being this issue. I wonder if adding a check in this block to query for the given revision would be sufficient enough to detect this case and return a more sensible error message. I can submit a diff for that if that'd be acceptable
Thu, Feb 2
Jan 23 2017
@yelirekim I think we're good here. Just needed to implement getHarbormasterPublishablePHID() for SubmitQueueIntegrationPlans, ReleaseCandidates, and ScheduledBuilds. See our downstream diff for those changes.
Jan 20 2017
Jan 19 2017
Jan 16 2017
ah yep that seemed to work. I was still specifying releaseNotes for the field which I'm guessing is why it wasn't getting saved. Thanks again for all the help! If you're ever in Pittsburgh I think I owe you about 50 beers at this point.
I pulled in that change and I'm still seeing the same issue. Editing the field from /differential/revision/edit/... seems to work normally, but adding it via the differential.createrevision endpoint does not.
Checking in for guidance in case this is the wrong way to go. I have a custom field which extends DifferentialStoredCustomField and I'm trying to convert it in the same way as the above. Thus far my field isn't being saved when calling differential.createrevision. I've included the DifferentialCommitMessageCustomField and the DifferentialStoredCustomField below.
Jan 11 2017
Thanks that did it! Still working through an issue with one of our more *ahem* exotic applications, but I suspect that'll be a more involved refactor on our end.
Jan 10 2017
Doing that without defining getFieldTransactions() in my class yields this exception:
Transaction with key "1" has invalid type "devTestPHIDs". This type is not recognized. Valid types are: update, comment, title, summary, testPlan, reviewers.add, reviewers.remove, reviewers.set, repositoryPHID, tasks.add, tasks.remove, tasks.set, view, edit, projects.add, projects.remove, projects.set, subscribers.add, subscribers.remove, subscribers.set, phabricator:auditors.
Thanks for the help Evan. I've gone through and converted everything as you suggested. The conduit method then started telling me
Jan 9 2017
Dec 9 2016
Choosing "Abandon Revision" because "Abandon Revision, Set it on Fire, and Pretend it Never Existed" isn't an option. I'll get a diff ready that adds that as an option.
Oh god. I step away for 5 minutes and I'm now on the phacility hit-list.
haha I had a feeling this probably fell into the category of things that are in the process of being done a better/holistic way
Dec 6 2016
Dec 5 2016
Nov 22 2016
Use <= rather than <
@epriestley do these changes look alright? I pulled most of PhabricatorWorkerArchiveTaskQuery up into the abstract PhabricatorWorkerTaskQuery and then added PhabricatorWorkerActiveTaskQuery. That made it more straightforward to just AND all the constraints together.
Backed PhabricatorWorkerActiveTasks with an actual query object and refactored to share
some logic between Active and Archived tasks. Also removed some overeager indentations.
Nov 21 2016
Confirmed this on my local. It seems to be due to D16851, specifically this line: https://secure.phabricator.com/source/phabricator/browse/master/src/applications/diffusion/application/PhabricatorDiffusionApplication.php$104.
Nov 10 2016
Pretty sure this works as intended
Yes I did have the persistent chat pane open. Also can't seem to repro without it.
The other errors that appear in DarkConsole:
Oct 28 2016
Oct 14 2016
Oct 13 2016
Hmmm I'm not seeing those lines in that file anywhere. On the current head of master, I'm pretty sure the viewer is always set to PhabricatorUser::getOmnipotentUser() in TransactionPublishWorker. Although maybe that isn't making it all the way to the policy aware query in every case.
Oct 12 2016
@jessjohnson I still haven't had a chance to dig further into this, but I'll report back when I do (or if you figure anything out, please let me know). I don't think killing off the tasks permanently is really an option since this seems to happen for specific degenerate instances of otherwise-needed tasks. Perhaps not allowing these tasks to retry would be an okay bandaid, but I'd still like to figure out the underlying problem if we can.
Oct 11 2016
Nope we don't have that integrated in yet (just missed our last cutoff). Once we get that pulled in I'll confirm it clears up the issue.
Evan, do you think there is a route forward with this that would be feasible for me to implement upstream?
Oct 6 2016
Oct 5 2016
Ah yeah it could be related to that (policy errors/unit test failures), although I wouldn't really describe this as "extremely rare" (except of course when I'm trying to make it happen). Some stats from grepping through logs: since last week it looks like this error has happened 73k times involving 193 distinct diffs. It's like the daemons are laughing at my inability to repro it.
Just spent a bit of time trying to repro this in a script but could not. The diffs always load successfully from the script. I even tried waiting until right after one of the errors popped up in the logs and then queried for that diff via the script right away.
Sep 28 2016
Gonna go ahead and say no one should be automatically subscribed to tokens
Fixed issues brought up in CR
Sep 27 2016
removed another vestige of application names
DELETE MOARRR CODE