Add Slack to Owners Page (Proof of Concept, do not land) #872
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…a metrics Summary: this diff adds exponential backoff retries for harbormaster builds that return an error status code (for our use case, this will usually be 28 for timeouts). in addition, it adds grafana metrics for HTTP post requests and their status codes (filterable by build plan). Test Plan: i copy pasted my local code onto the build-core phabricator code base and then used the bin/harbormaster binary to manually trigger runs of my build plan. you can see it in action here: https://phabricator.robinhood.com/harbormaster/build/2385585/ i intentionally changed the TOKEN for one of the jobs to something different, and as you can see the HTTP403 was retried 3 times at increasing intervals. the other job which i didnt modify was triggered succesfully and ran to completion. you can also see associated metrics here: http://metrics.dev.rhinternal.net/explore?orgId=1&left=%5B%22now-1h%22,%22now%22,%22cortex-multitenant%22,%7B%22exemplar%22:true,%22expr%22:%22phabricator_build_result_total%22%7D%5D Reviewers: paul.tarjan, stephan.zharkov, harry.li Reviewed By: paul.tarjan Task Link: https://robinhood.atlassian.net/browse/TI-1144 Differential Revision: https://phabricator.robinhood.com/D292841
Summary:
To handle the security task, we've decided to disable 'foist upon' and
'commandeer' feature in our fork.
Test Plan:
tested in staging.
* Before:
revision editor: {F5071084}
author's UI: {F5071086}
reviewer's UI: {F5071087}
* After:
revision editor: {F5071088}
author's UI: {F5071154}
reviewer's UI: {F5071089}
Reviewers: greg.magolan, paul.tarjan
Reviewed By: paul.tarjan
Task Link: https://robinhood.atlassian.net/browse/DEVX-2064
Differential Revision: https://phabricator.robinhood.com/D304649
Summary: Try to improve the revision page load latency. Jenkins test will report back to harbormaster for every single test case run, and phabricator will save all of them to database. Currently the table is having 1.8 billion rows, and for the revision that has a lot of test cases in CI, the page load logic will read huge amount of data, and often we see timeouts. After this change, we will only save for failed test case (so that dev can still view the test log), and if coverage data is provided in the unit_message, we will save as well. Test Plan: manual apply change to staging and test Reviewers: boyang.tian Reviewed By: boyang.tian Task Link: DEVX-2087 Differential Revision: https://phabricator.robinhood.com/D306789
Summary: Phabricator is checking files' policy if they are used in timeline or feed. For commonly used files, like a meme in the test failure message, the file will be attached to huge amount of the revisions, and make the policy check very slow and resource hogging. This change adds a short-circuit evaluation: if file policy is public or all_users, skip doing the expensive check. Test Plan: Change in staging and test. Reviewers: boyang.tian Reviewed By: boyang.tian Differential Revision: https://phabricator.robinhood.com/D306850
Summary: Based on the slack discussion: https://hood.slack.com/archives/C016GGP0QDD/p1635800664006300 Partially revert 386590f to restore commandeer. Please notice that with commandeer, hacker can use single machine to land the code: grabbing an accepted diff -> change -> land. Test Plan: staging Reviewers: paul.tarjan Reviewed By: paul.tarjan Subscribers: paul.tarjan Differential Revision: https://phabricator.robinhood.com/D307232
Summary: ^ Test Plan: i can patch in the code on canary phab and make sure harboramster is still good tested live on prod phabricator as well Reviewers: paul.tarjan Reviewed By: paul.tarjan Differential Revision: https://phabricator.robinhood.com/D303561
Summary: We want to ensure that Phabricator username is the same as user's email prefix and not changed during registration. SW-ID-6553158c3b Test Plan: Observe registration flow after deployment with an employee who is not an engineer Reviewers: gabriel.silk, paul.tarjan Reviewed By: gabriel.silk Differential Revision: https://phabricator.robinhood.com/D348202
Summary: The previous change did disable the box, but resulted in an error after clicking the register button afterwards. This is attempt to do this differently - using another control type. SW-ID-a8c6c5308e Test Plan: Observe registration flow after deployment with an employee who is not an engineer Reviewers: gabriel.silk, paul.tarjan Reviewed By: gabriel.silk Differential Revision: https://phabricator.robinhood.com/D349374
Summary: I understood this controller calls this page multiple times, so this change takes this into account. SW-ID-d68578809d Test Plan: Observe registration flow after deployment with an employee who is not an engineer Reviewers: gabriel.silk, paul.tarjan Reviewed By: gabriel.silk Differential Revision: https://phabricator.robinhood.com/D350590
Summary: ^ SW-ID-69834bc741 Test Plan: None Reviewers: david.aghassi Reviewed By: david.aghassi Differential Revision: https://phabricator.robinhood.com/D428019
Summary: ^ SW-ID-4df721d987 Test Plan: staging Reviewers: david.aghassi Reviewed By: david.aghassi Differential Revision: https://phabricator.robinhood.com/D431131
Summary: Stale revisions are unlandable and make it very hard to find things in the UI. SW-ID-f8f44e04f7 Test Plan: staging Reviewers: anna.pstrucha Reviewed By: anna.pstrucha Differential Revision: https://phabricator.robinhood.com/D447876
Summary: This will help manage the large number of stale and abandoned diffs left after the layoffs. SW-ID-b9431677ef Test Plan: staging Reviewers: anna.pstrucha Reviewed By: anna.pstrucha Differential Revision: https://phabricator.robinhood.com/D447877
Summary: Phabricator alters html symbols like < and > before rule processing which prevents the current diclosure rule from ever being applied. This changes the rule to use the syntax of confluences expand macro to avoid using html symbols. Test Plan: local testing Differential Revision: https://phabricator.robinhood.com/D475229
Summary: This is expected to include project tags in the commit message that is fetched by PHLQ Test Plan: Deploy to Phabricator staging and use https://secure.phabricator.com/conduit/method/differential.getcommitmessage/ to view rendering Reviewers: #devx Subscribers: Task Link:
[commit-rendering] include project tags in commit template
Summary:
when we query revisions, there is `exact(<username>)` function that matches the exact user only, and not projects or groups the user is a member of; this is useful when we search revisions that we are directly tagged on, however this function must take in a specific username, so I can't build a query generalized for everyone
there is also a `viewer()` function, which is supposed to be a general query that can be used to build dashboards, so whoever views a dashboard used query with this function only sees their own stuff, but this includes the viewer's projects and groups and everything
this diffs adds an `exact-viewer()` function, which combines the two, so we can query things match exactly the viewer
why this change: when we discussed in our team sync how the current default phabricator "Home" dashboard is not so useful and I built my own query, @brian.myers said he wanted it to. So I figure instead of teaching people the query, I can just build a new home dashboard that's more informational. this exact-viewer() function makes the query generalized so I can just build a dashboard on top this query and make it available to everyone
Test Plan:
tested locally, and on phabricator staging
when I query "exact current viewer"
{F14482326}
I get my own stuff
{F14482342}
when I query "current viewer"
{F14482353}
I get a diff where I'm the group reviewer
{F14482375}
Reviewers: brian.myers, joel.jeske
Reviewed By: brian.myers
Subscribers: brian.myers
Differential Revision: https://phabricator.robinhood.com/D643179
…object [custom fields] Call setObject before shouldEnableForRole
Summary: When engineers know which channel to ping for review, reviews are done faster. For every 5 minutes it takes to do a code review costs 1600 engineer days are saved per year. Test Plan: Demo Reviewers: Subscribers: Task Link: RedOak ID: Tags:
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.