20after4 (Mukunda Modell)
Release Engineer @ Wikimedia Foundation

Projects

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Monday

  • Clear sailing ahead.

User Details

User Since
Nov 28 2011, 9:35 AM (290 w, 4 d)
Availability
Available

Recent Activity

Tue, Jun 20

20after4 awarded T12855: In PHP7, "Throwable" and "Error" are exciting new exception classes a Baby Tequila token.
Tue, Jun 20, 8:12 PM · Infrastructure
20after4 added a comment to T920: Provide SMS Support.

https://www.wired.com/2016/06/hey-stop-using-texts-two-factor-authentication/

Tue, Jun 20, 7:44 AM · Restricted Project, Herald

Sun, Jun 18

20after4 created D18132: Let PhabricatorSearchCheckboxesField survive saved query data with mismatched types.
Sun, Jun 18, 8:16 AM
20after4 created T12851: The feed query '2HfUlTnoqZbw' causes an exception in PhabricatorSearchCheckboxesField.
Sun, Jun 18, 8:14 AM · Feed, Bug Report

Fri, Jun 16

20after4 added a comment to T12799: Consider an "API Utilities" application.

That does sound powerful. +1

Fri, Jun 16, 8:41 AM · Conduit
20after4 awarded T11439: Retrieve Diff PHID via phid.lookup a Love token.
Fri, Jun 16, 8:30 AM · Feature Request

Wed, Jun 14

20after4 added a comment to T12799: Consider an "API Utilities" application.

I have spent a little time poking at code, trying to figure out how to build a simple template engine into the description field, so that you can reference hidden custom fields by {field-identifier} or similar from within the remarkup. I resigned myself to building something just like publish.php, where the descriptions are maintained via conduit, however, I still long for it to be built in.

Wed, Jun 14, 3:14 PM · Conduit
20after4 added a comment to T12314: Support formal task subtypes (like "bug" vs "feature").

It would be really nice to default newly added fields to hidden. Going through 20+ forms to hide the fields is tedious.

Wed, Jun 14, 3:06 PM · EditEngine, Maniphest, Custom Fields, Feature Request

Tue, Jun 13

20after4 added a comment to D18121: Allow dashboard panels to be found by monogram.

Indeed, I tested this and removing the setDisplayName retains the correct behavior (show the monogram AND let you search by monogram)

Tue, Jun 13, 9:49 PM
20after4 committed rPe1850b3c4e4e: Allow dashboard panels to be found by monogram (authored by 20after4).
Allow dashboard panels to be found by monogram
Tue, Jun 13, 9:48 PM
20after4 closed D18121: Allow dashboard panels to be found by monogram by committing rPe1850b3c4e4e: Allow dashboard panels to be found by monogram.
Tue, Jun 13, 9:48 PM
20after4 added a comment to D18121: Allow dashboard panels to be found by monogram.

This seems light it was probably just an oversight and it's a trivial change so I'm upstreaming. Already in queue to be deployed downstream in Wikimedia's install.

Tue, Jun 13, 9:44 PM
20after4 created D18121: Allow dashboard panels to be found by monogram.
Tue, Jun 13, 9:43 PM

Sat, Jun 10

20after4 added a comment to T7593: Allow administrators to disable files to prevent "l33t w4r3z" abuse cases.

FWIW we have seen several users attempting to distribute l33t w4r3z via Wikimedia's instance of Phabricator. I had to set file upload limits to < 8MB in order to prevent chunked file storage.

Sat, Jun 10, 12:36 AM · Abuse, Files
20after4 added a comment to T12799: Consider an "API Utilities" application.

This sounds REALLY useful. Relatedly, it would be useful if I could auto-generate a task for each recurrence of an event.

Sat, Jun 10, 12:29 AM · Conduit
20after4 added a comment to T12819: InnoDB FULLTEXT appears to fail catastrophically once it reaches a moderate size.

:(

Sat, Jun 10, 12:12 AM · Search

Wed, Jun 7

20after4 added a comment to T12804: Diffusion pre-redesign UI feedback (June 2017).

You can use, for example, -C3 with git grep to show 3 lines of Context around each match.

About 95% of the time when I'm searching for content it's because I want to edit a subset of the callsites. I do this with git grep some_function | maybe_more_grep | give, which opens all matches in my editor. This would take me about 100 years from the web UI for a nontrivial number of results. Do you have some other use case for searching for content?

Wed, Jun 7, 9:24 PM · Diffusion, Design & Planning
20after4 added a comment to T12804: Diffusion pre-redesign UI feedback (June 2017).

When I view a repository, it would be incredibly helpful if the files I most recently touched were at the top of the UI.

Wed, Jun 7, 9:10 PM · Diffusion, Design & Planning
20after4 added a comment to T12804: Diffusion pre-redesign UI feedback (June 2017).

What I would really love to see is global filename search. Currently you can search for a filename within one repository but you have to know which repo to look in first.

Wed, Jun 7, 8:24 PM · Diffusion, Design & Planning
20after4 added a comment to T12804: Diffusion pre-redesign UI feedback (June 2017).

We have thousands of repositories and it's hard to differentiate in a list which repository I need.

Wed, Jun 7, 8:21 PM · Diffusion, Design & Planning

May 22 2017

20after4 added a comment to T8646: Provide more context for search results, particularly wiki documents.

FWIW I partially implemented this in Wikimedia's fork, and I did so in a reusable way. I'd like to eventually upstream it but I'm not sure that my approach is desired upstream. I'll give it another shot though.

May 22 2017, 3:51 AM · Design & Planning, Search, Restricted Project
20after4 abandoned D13581: Support custom fields in diffusion commit property views..

not upstream-able.

May 22 2017, 3:46 AM
20after4 added a comment to D17615: Don't apply offset to elasticsearch query.

Yeah I think we are good now.

May 22 2017, 3:46 AM
20after4 abandoned D17615: Don't apply offset to elasticsearch query.
May 22 2017, 3:46 AM
20after4 awarded T12733: (2017 Week 20) Inline Comments Errata / Feedback a Like token.
May 22 2017, 2:25 AM · Inline Comments, Installing & Upgrading, Differential

May 8 2017

20after4 added a comment to T12681: Upcoming Support Product / End of Paid Prioritization.

phabricator trading cards ftw.

May 8 2017, 6:21 PM · Guides, Phacility
20after4 added a comment to T12681: Upcoming Support Product / End of Paid Prioritization.

I think that @epriestley just made capitalism and socialism obsolete by re-imagining everything in terms of Mana points. You sir, have won the internet.

May 8 2017, 5:52 PM · Guides, Phacility
20after4 awarded T8236: `arc weld` should do something a Pirate Logo token.
May 8 2017, 5:46 PM · Contributor Onboarding, Arcanist
20after4 added a comment to T8236: `arc weld` should do something.

This should clearly be priority: Unbreak now!

May 8 2017, 5:45 PM · Contributor Onboarding, Arcanist

May 2 2017

20after4 awarded T12003: Explain to users how fulltext queries are parsed and executed a Love token.
May 2 2017, 9:27 PM · Search
20after4 abandoned D17608: Add highlighting support to Elasticsearch fulltext engine.

@epriestley: I'll submit a new revision that expands on the resultset class as I mentioned above. Of course this is without any expectations regarding when you'll have time to look at it :)

May 2 2017, 9:26 PM

Apr 26 2017

20after4 added a comment to T12314: Support formal task subtypes (like "bug" vs "feature").

Changing the type is going to run into the issue of what to do about the fields which differ between the two types. Fields which are present in the old type will continue to be displayed unless you clear them when changing types.

Apr 26 2017, 3:47 PM · EditEngine, Maniphest, Custom Fields, Feature Request

Apr 24 2017

20after4 added a comment to D17608: Add highlighting support to Elasticsearch fulltext engine.

@epriestley: Given that this is an elastic-only feature and not really wanted upstream, and given that you have implemented result sets as objects in rP3245e74f16bb: Show users how fulltext search queries are parsed and executed; don't query…, I think I should abandon this and submit a new revision with just the PhabricatorSearchResultEngineExtension infrastructure.

Apr 24 2017, 10:05 PM

Apr 8 2017

20after4 accepted rP699ab153e375: (stable) Promote 2017 Week 14.
Apr 8 2017, 6:16 PM

Apr 7 2017

20after4 added a comment to D17608: Add highlighting support to Elasticsearch fulltext engine.

I think this approach is at least somewhere in the realm of reasonable, yeah.

The extensions should probably get passed a list of all results at once, not individual results, so they can bulk-load additional data (for example, hierarchies for wiki documents or thumbnails for files or whatever) instead of needing to do 100 queries to show 100 items.

So maybe the API ends up looking like $extension->messAroundWithTheseViewsBeforeWeShowThemToTheUserIfYouWant($items), but general flow seems reasonable to me.

Apr 7 2017, 9:55 AM

Apr 5 2017

20after4 planned changes to D17608: Add highlighting support to Elasticsearch fulltext engine.

Warning: This code is still a bit messy, I will clean things up a bit more in a future diff.

Apr 5 2017, 7:03 AM
20after4 updated the diff for D17608: Add highlighting support to Elasticsearch fulltext engine.

Significant refactor to use a PhabricatorFulltextResult object to represent search
result hits. Results views are made extensible using the EngineExtension pattern:

Apr 5 2017, 6:57 AM

Apr 4 2017

20after4 added a comment to D17615: Don't apply offset to elasticsearch query.

Just to see what would happen, I tried returning 100 dummy results + the real results. That didn't seem to satisfy the pager, so perhaps I'm wrong about what's actually happening. I spent quite a bit of time tracing through PhabricatorPolicyAwareQuery and still didn't ascertain exactly what is going on, however, it seems like it's skipping the results in PhabricatorPolicyAwareQuery::execute() right around line 211

Apr 4 2017, 5:44 PM
20after4 added a comment to D17615: Don't apply offset to elasticsearch query.

With the HEAD behavior we return 100 results for offset = 700, limit = 100 but with the proposed behavior we return 800, which is a technical argument for fixing this somewhere above the engine in the stack.

Apr 4 2017, 5:35 PM
20after4 added a comment to D17615: Don't apply offset to elasticsearch query.

The problem is that we do pagination at a higher layer and the cursor skips 100 results when the offset is set to 100. So if we also apply an offset to elastic, then we skip 100 results in two places, thus skipping to the 200th result.

Apr 4 2017, 5:33 PM
20after4 added a task to D17615: Don't apply offset to elasticsearch query: T12450: New Search Configuration Errata.
Apr 4 2017, 5:18 PM
20after4 added a revision to T12450: New Search Configuration Errata: D17615: Don't apply offset to elasticsearch query.
Apr 4 2017, 5:18 PM · Search
20after4 updated the diff for D17615: Don't apply offset to elasticsearch query.

track upstream branch so arcanist can push to staging.

Apr 4 2017, 5:17 PM
20after4 created D17615: Don't apply offset to elasticsearch query.
Apr 4 2017, 5:15 PM
20after4 added a revision to T8285: Global advanced search only shows max. 100 results; pagination link ("Next") shown but empty: D17615: Don't apply offset to elasticsearch query.
Apr 4 2017, 5:15 PM · Search
20after4 added 1 mock(s) for T8646: Provide more context for search results, particularly wiki documents: M1474: Show snippets in search results....
Apr 4 2017, 4:40 PM · Design & Planning, Search, Restricted Project
20after4 added 1 task(s) for M1474: Show snippets in search results...: T8646: Provide more context for search results, particularly wiki documents.
Apr 4 2017, 4:40 PM · Search
20after4 created M1474: Show snippets in search results....
Apr 4 2017, 4:39 PM · Search
20after4 added a comment to D17608: Add highlighting support to Elasticsearch fulltext engine.

I lean toward having this return a SearchResult kind of object which has methods like getHandle() and getBodySnippet() and whatever. I think we need this for T8646 anyway. It doesn't make sense to put stuff like "breadcrumb hierarchy" or "thumbnail document preview" or whatever on handles, since there's like a 99% chance that only global search results will ever use them.

Apr 4 2017, 4:15 PM
20after4 added a comment to D17608: Add highlighting support to Elasticsearch fulltext engine.

Finally, it overrides subtitles on handles which really have them. Today I think this is only Users, but could be more types of objects in the future.

Apr 4 2017, 3:59 PM
20after4 added a comment to D17608: Add highlighting support to Elasticsearch fulltext engine.

I'm not really comfortable trusting Elasticsearch to return a safe blob of HTML here: it seems like we're putting a lot of trust in it by doing this (willingly applying PhutilSafeHTML() to anything it returns) but not really getting very much benefit (basically just some highlighting, which we already have code for elsewhere) relative to the level of trust involved.

In theory there's almost no possible way Elasticsearch could get things wrong since the task is so simple, and I don't think this is likely to ever lead to a security issue, it just creates a very large amount of new attack surface which we don't really need to have.

Apr 4 2017, 3:54 PM
20after4 created D17608: Add highlighting support to Elasticsearch fulltext engine.
Apr 4 2017, 2:10 AM
20after4 added a revision to T12500: Render highlighted search result body snippets in global search results views: D17608: Add highlighting support to Elasticsearch fulltext engine.
Apr 4 2017, 2:10 AM · Elasticsearch, Search, Feature Request

Apr 3 2017

20after4 created T12500: Render highlighted search result body snippets in global search results views.
Apr 3 2017, 5:41 PM · Elasticsearch, Search, Feature Request
20after4 added a comment to T7860: When users search for "#x y", treat "#x" as a project hashtag.

I implemented this recently but then pulled it out because ... project slug lookup is a gigantic mess right now.

Apr 3 2017, 3:16 PM · Search
20after4 awarded D17596: Remove FIELD_KEYWORDS, index project slugs as body content a Like token.
Apr 3 2017, 2:50 PM
20after4 added a comment to T8238: Formally support side-band change handoff in external repositories.

@dreadlord2203 indeed, hiding those tags would be useful. I found one problem with using a separate repository for change handoff, and that is access control. If you maintain several repositories with different access controls, then you would have to duplicate each of them and maintain a second set of access controls for the corresponding staging repositories. This is a lot of extra work and it would be easy to make a mistake and leak the staging repo to people who shouldn't be able to see it.

Apr 3 2017, 2:48 PM · Restricted Project, Restricted Project, Diffusion, Harbormaster
20after4 awarded T12493: Upgrading: Fulltext Search Services a Love token.
Apr 3 2017, 2:44 PM · Elasticsearch, Search, Guides

Apr 2 2017

20after4 added a comment to T12450: New Search Configuration Errata.

Maybe let bin/search commands target a specific service.

Apr 2 2017, 8:56 PM · Search
20after4 added a comment to T12450: New Search Configuration Errata.
It would be nice to build a practical test suite instead, where we put specific documents into the index and then search for them.
Apr 2 2017, 8:49 PM · Search
20after4 accepted D17597: Count and report skipped documents from "bin/search index".

It's definitely not obvious that objects are skipped based on the object version rather than what's in the index. So yeah this is helpful.

Apr 2 2017, 8:44 PM
20after4 accepted D17598: When "cluster.search" changes, don't trust the old index versions.

Good idea. Definitely much easier than storing per-index version numbers and trying to keep those in sync.

Apr 2 2017, 8:41 PM
20after4 accepted D17599: After a fulltext write to a particular service fails, keep trying writes to other services.

Yes this seems reasonable.

Apr 2 2017, 8:39 PM
20after4 added a comment to D17600: Remove "url" from Elasticsearch index.

plustwo

Apr 2 2017, 8:37 PM
20after4 added a comment to D17602: Adjust and wordsmith Search documentation.

Really try to avoid suggesting anyone configure Elasticsearch ever for any reason.

Apr 2 2017, 8:09 PM
20after4 accepted D17602: Adjust and wordsmith Search documentation.

nice! Big improvement over my initial document.

Apr 2 2017, 8:07 PM

Mar 30 2017

20after4 updated the task description for T12450: New Search Configuration Errata.
Mar 30 2017, 6:11 PM · Search
20after4 added a comment to T12450: New Search Configuration Errata.

So I ran into one problem when deploying the latest code to WMF production. We now throw a setup error if we have a cluster with no readable hosts. It actually makes sense to have a cluster that is write-only so that setup error is bogus.

Mar 30 2017, 6:10 PM · Search
20after4 added a task to D17580: Set content-type to application/json: T12450: New Search Configuration Errata.
Mar 30 2017, 6:09 PM
20after4 added a revision to T12450: New Search Configuration Errata: D17580: Set content-type to application/json.
Mar 30 2017, 6:09 PM · Search
20after4 added a revision to T12450: New Search Configuration Errata: D17581: Make sure writes go to the right cluster.
Mar 30 2017, 6:09 PM · Search
20after4 added a task to D17581: Make sure writes go to the right cluster: T12450: New Search Configuration Errata.
Mar 30 2017, 6:09 PM
20after4 committed rPcb1d90465447: Make sure writes go to the right cluster (authored by 20after4).
Make sure writes go to the right cluster
Mar 30 2017, 6:08 PM
20after4 closed D17581: Make sure writes go to the right cluster by committing rPcb1d90465447: Make sure writes go to the right cluster.
Mar 30 2017, 6:08 PM
20after4 committed rP67a1c4047647: Set content-type to application/json (authored by 20after4).
Set content-type to application/json
Mar 30 2017, 6:07 PM
20after4 closed D17580: Set content-type to application/json by committing rP67a1c4047647: Set content-type to application/json.
Mar 30 2017, 6:07 PM
20after4 added a comment to D17575: Provide some guidance about elasticsearch in cluster docs.

I figured out that one component of the WMF scaling issue was caused by the very pathological case of searching for a word that appears in very many documents, exacerbated by lots simultaneous queries fired off from users repeatedly searching from the typeahead in the related tasks editor. I agree that we still don't know the exact cause of the 100x slowdown.

Mar 30 2017, 6:05 PM
20after4 added a comment to T12314: Support formal task subtypes (like "bug" vs "feature").

@cos: probably should not have a default value.

Mar 30 2017, 5:21 PM · EditEngine, Maniphest, Custom Fields, Feature Request
Ruthvika awarded rP654f0f6043f8: Make messages translatable and more sensible. a Like token.
Mar 30 2017, 9:56 AM
20after4 created D17581: Make sure writes go to the right cluster.
Mar 30 2017, 2:31 AM

Mar 29 2017

20after4 updated the diff for D17580: Set content-type to application/json.

Don't use $future->write, that doesn't work in all cases

Mar 29 2017, 11:48 PM
20after4 created D17580: Set content-type to application/json.
Mar 29 2017, 9:48 PM

Mar 28 2017

20after4 committed rP654f0f6043f8: Make messages translatable and more sensible. (authored by 20after4).
Make messages translatable and more sensible.
Mar 28 2017, 11:17 PM
20after4 closed D17578: Make messages translatable and more sensible. by committing rP654f0f6043f8: Make messages translatable and more sensible..
Mar 28 2017, 11:17 PM
20after4 created D17578: Make messages translatable and more sensible..
Mar 28 2017, 11:14 PM
20after4 accepted rP8879118b696f: Fix a mid-air collision around SearchService roles.
Mar 28 2017, 10:13 PM
20after4 added a task to D17575: Provide some guidance about elasticsearch in cluster docs: T12450: New Search Configuration Errata.
Mar 28 2017, 10:07 PM
20after4 added a revision to T12450: New Search Configuration Errata: D17575: Provide some guidance about elasticsearch in cluster docs.
Mar 28 2017, 10:07 PM · Search
20after4 created D17575: Provide some guidance about elasticsearch in cluster docs.
Mar 28 2017, 10:06 PM
20after4 accepted D17574: Re-run config validation from `bin/search`.
Mar 28 2017, 9:46 PM
20after4 committed rP699228c73b74: Address some New Search Configuration Errata (authored by 20after4).
Address some New Search Configuration Errata
Mar 28 2017, 8:19 PM
20after4 added a commit to T12450: New Search Configuration Errata: rP699228c73b74: Address some New Search Configuration Errata.
Mar 28 2017, 8:19 PM · Search
20after4 closed D17564: Address some New Search Configuration Errata by committing rP699228c73b74: Address some New Search Configuration Errata.
Mar 28 2017, 8:19 PM
20after4 added a comment to D17564: Address some New Search Configuration Errata.

Just as a general workflow suggestion, I'd encourage you to do this as a bunch of small changes instead of one big "fix everything" change

Mar 28 2017, 8:16 PM
20after4 added a comment to D17572: Make `bin/search init` messaging a little more consistent.

Seems legit.

Mar 28 2017, 8:00 PM
20after4 accepted D17572: Make `bin/search init` messaging a little more consistent.
Mar 28 2017, 8:00 PM
20after4 accepted D17573: Remove PhabricatorSearchEngineTestCase.
Mar 28 2017, 7:59 PM
20after4 accepted D17571: Fix isReadable() and isWritable() in SearchService.
Mar 28 2017, 7:58 PM
20after4 committed rP9e2f263bb49c: Add repositories to fulltext search index. (authored by 20after4).
Add repositories to fulltext search index.
Mar 28 2017, 7:58 AM
20after4 closed D17300: Add repositories to fulltext search index. by committing rP9e2f263bb49c: Add repositories to fulltext search index..
Mar 28 2017, 7:58 AM · Diffusion, Search