Page MenuHomePhabricator

Flow: API module issues
Closed, ResolvedPublic

Description

For action=flow:

  • The "flowaction" parameter lacks a description.
  • The "page" parameter lacks a description.
  • The "flowaction" parameter does not list available actions.
  • The "params" parameter is poorly documented. Passing a json blob of "params" is rather awful. I might rather see a setup where "flowaction" selects a submodule, along the lines of how action=query works, so these parameters could actually be documented in api.php.
  • getExamples() should be properly implemented.
  • getHelpUrls() should be implemented.
  • I'm assuming at least some of the possible flowactions are write actions. isWriteMode() should therefore return true.
  • I note that ApiBase is already a ContextSource, so there is no need for $this->getContext()->getUser() instead of just $this->getUser().
  • I see you're setting "_element" directly. Use ApiResult's method to handle that instead, please. The same goes if you're setting "*" directly anywhere.
  • I see that sometimes the "result" element in the result will be an associative array and sometimes it will be the string "error". I'd rather see a "status" element of some sort that is always a string and have "result" always be an associative array when it is present.
  • I see you have a "doRenderer" protected method that is never actually called.

For list=flow:

  • The link returned by getHelpUrls() does not actually document this API module.
  • The "flowaction" parameter does not list available actions.
  • Same criticisms of the "params" blob as above.
  • You are setting "_element" directly instead of using ApiResult's methods.
  • You should be checking the return from ApiResult's addValue method and properly handling failure.

Version: master
Severity: normal

Details

Reference
bz57659

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 2:24 AM
bzimport set Reference to bz57659.

bingle-admin wrote:

The WMF core features team tracks this bug on Mingle card https://mingle.corp.wikimedia.org/projects/flow/cards/532, but people from the community are welcome to contribute here and in Gerrit.

Marking as high priority, it gets much harder to change the API once something is deployed...

(In reply to comment #0)

  • I note that ApiBase is already a ContextSource, so there is no need for

$this->getContext()->getUser() instead of just $this->getUser().

https://gerrit.wikimedia.org/r/#/c/102046/

  • The link returned by getHelpUrls() does not actually document this API

module.

Removed in https://gerrit.wikimedia.org/r/#/c/102030/

(In reply to comment #3)

(In reply to comment #0)

  • The link returned by getHelpUrls() does not actually document this API

module.

Removed in https://gerrit.wikimedia.org/r/#/c/102030/

Good to get rid of the bad link. But list=flow does need a valid implementation of getHelpUrls(). I expect you already are aware of this, but the obvious bears stating.

Flow team: This was reported in November with pretty clear concerns. Brad is the current point of reason/arbitration for the API. Can you give a response to this as well? (On BZ not Mingle ;) ).

Change 107411 had a related patch set uploaded by Legoktm:
[WIP] Revamp API

https://gerrit.wikimedia.org/r/107411

Brad,

We've chosen our first deployment to English Wikipedia with care. The reason Flow is only going live on the 2 discussion pages in question (Wikipedia_talk:Wikiproject_Breakfast and Wikipedia_talk:Wikiproject_Hampshire) is that those pages have zero bot activity, and relatively light human traffic, for that matter, skewing largely power user (e.g. no clueless n00bs who need to get smacked around by Cluebot). In my capacity as product owner, I felt completely comfortable deploying a beta trial of experimental software with an incomplete API to two pages where an API wouldn't be critically necessary for vital workflows, with the understanding that the next step after this deployment would be to reach out to bot operators, invite them to test aggressively on a developer test page, and work with them to make the API fulfill their needs. I apologize for not communicating all of this to you explicitly sooner; frankly, it was hard for me to ascertain whether this particular bug was more or less high-priority than your critiques of our use of whitespace. In the future, let's not rely on passive Bugzilla communication around things you feel are serious blockers; let's just talk to each other directly -- I don't bite :)

I understand that your priorities as a platform engineer fall toward making sure our API is robust and well-documented. But please keep in mind that this project also involves significant design and UI components that are just as critical to the success of the overall mission (to replace talk pages entirely and create more universally-usable, stable software tools for all the processes they support). Putting Flow out there on a couple of real discussion spaces is the only way we're going to be able to get actionable feedback on the design and UI side. The sooner we start receiving this kind of feedback, the sooner we can prioritize our work on all the outstanding bugs, enhancements and feature requests, so we get the right features to the right users at the right time.

(In reply to comment #7)

Putting Flow out there on a couple of real discussion spaces
is
the only way we're going to be able to get actionable feedback on the design
and UI side. The sooner we start receiving this kind of feedback, the sooner
we
can prioritize our work on all the outstanding bugs, enhancements and feature
requests, so we get the right features to the right users at the right time.

That sounds like the same reasoning they used when deploying VE to enwiki with known bugs unfixed. I think we all know how well *that* went.

I realize the proposed deployment of Flow to enwiki is somewhat different since it's a deployment to two rather out-of-the-way talk pages rather than being enabled by default for everyone. But on the other hand, it's also that much less likely to give you the sort of feedback you're hoping for. And certain vocal community members will be on the defensive given the recent VE situation, ready to blast "the WMF" for doing the "same" thing again.

(In reply to comment #7)

I understand that your priorities as a platform engineer fall toward making
sure our API is robust and well-documented.

I also want to avoid causing our volunteer community members to develop towards an API with known major flaws by pushing it on a production site before it's ready. The test wikis and mediawiki.org, while in production, are still test sites.

(In reply to comment #8)

(In reply to comment #7)

Putting Flow out there on a couple of real discussion spaces
is
the only way we're going to be able to get actionable feedback on the design
and UI side. The sooner we start receiving this kind of feedback, the sooner
we
can prioritize our work on all the outstanding bugs, enhancements and feature
requests, so we get the right features to the right users at the right time.

That sounds like the same reasoning they used when deploying VE to enwiki
with
known bugs unfixed. I think we all know how well *that* went.

I realize the proposed deployment of Flow to enwiki is somewhat different
since
it's a deployment to two rather out-of-the-way talk pages rather than being
enabled by default for everyone. But on the other hand, it's also that much
less likely to give you the sort of feedback you're hoping for. And certain
vocal community members will be on the defensive given the recent VE
situation,
ready to blast "the WMF" for doing the "same" thing again.

Possibly. There are some community members who would find a way to take offence to "how is your day going?". I don't particularly think we should base our decisions around pandering to them.

(In reply to comment #7)

I understand that your priorities as a platform engineer fall toward making
sure our API is robust and well-documented.

I also want to avoid causing our volunteer community members to develop
towards
an API with known major flaws by pushing it on a production site before it's
ready. The test wikis and mediawiki.org, while in production, are still test
sites.

(In reply to comment #7)

In the future, let's not rely on passive Bugzilla communication
around things you feel are serious blockers; let's just talk to each other
directly

In that case please document outcome as a comment in Bugzilla, so community members can also be aware and provide input if they were not physically around in your office.

(In reply to comment #10)

(In reply to comment #7)

In the future, let's not rely on passive Bugzilla communication
around things you feel are serious blockers; let's just talk to each other
directly

In that case please document outcome as a comment in Bugzilla, so community
members can also be aware and provide input if they were not physically
around
in your office.

I think that's what Maryana just said she'd do. Indeed, it's what she just did :).

(In reply to comment #10)

(In reply to comment #7)

In the future, let's not rely on passive Bugzilla communication
around things you feel are serious blockers; let's just talk to each other
directly

In that case please document outcome as a comment in Bugzilla, so community
members can also be aware and provide input if they were not physically
around
in your office.

Personally, I find bugzilla works as well as email and IRC doesn't seem like it would have been that much of an improvement here.

No one besides me is around my office. ;)

(In reply to comment #12)

(In reply to comment #10)

(In reply to comment #7)

In the future, let's not rely on passive Bugzilla communication
around things you feel are serious blockers; let's just talk to each other
directly

In that case please document outcome as a comment in Bugzilla, so community
members can also be aware and provide input if they were not physically
around
in your office.

Personally, I find bugzilla works as well as email and IRC doesn't seem like
it
would have been that much of an improvement here.

No one besides me is around my office. ;)

Personally I find impersonal communication methods like bugzilla a passive aggressive way to air issues without actually talking to anyone directly. If you had come into our irc room months ago and xommunicated in real time I guarantee this bug would be in a different state.

(In reply to comment #13)

(In reply to comment #12)

(In reply to comment #10)

(In reply to comment #7)

In the future, let's not rely on passive Bugzilla communication
around things you feel are serious blockers; let's just talk to each other
directly

In that case please document outcome as a comment in Bugzilla, so community
members can also be aware and provide input if they were not physically
around
in your office.

Personally, I find bugzilla works as well as email and IRC doesn't seem like
it
would have been that much of an improvement here.

No one besides me is around my office. ;)

Personally I find impersonal communication methods like bugzilla a passive
aggressive way to air issues without actually talking to anyone directly. If
you had come into our irc room months ago and xommunicated in real time I
guarantee this bug would be in a different state.

Bugzilla is how one files bugs against a product--sorry you don't like it. The nice thing is it's asynchronous so if someone is in a different timezone they can easily report issues without waiting for you to be around :)

(In reply to comment #14)

Bugzilla is how one files bugs against a product--sorry you don't like it.
The
nice thing is it's asynchronous so if someone is in a different timezone they
can easily report issues without waiting for you to be around :)

Bugzilla is an excellent way to track bugs, but for solving problems nothing beats communicating with a human being in real time. The foundation uses real time communication within teams extensively to organize and ensure the work they want to get done happens. Embrace the most effective forms of communication you can.

Lucky for those in other timezones, we have team members in 2 american timezones, europe and australia.

(In reply to comment #15)

(In reply to comment #14)

Bugzilla is how one files bugs against a product--sorry you don't like it.
The
nice thing is it's asynchronous so if someone is in a different timezone they
can easily report issues without waiting for you to be around :)

Bugzilla is an excellent way to track bugs, but for solving problems nothing
beats communicating with a human being in real time.

Indeed. But it's also good to file bugs because people are imperfect and can forget conversations they have :)

The foundation uses
real
time communication within teams extensively to organize and ensure the work
they want to get done happens. Embrace the most effective forms of
communication you can.

I know how the Foundation works...I work here too ;-)

Is real time communication awesome? Sure. Is finding someone on IRC to chat about your problem sometimes faster than a bug? Absolutely.

But you said:

(In reply to comment #13)

Personally I find impersonal communication methods like bugzilla a passive
aggressive way to air issues without actually talking to anyone directly. If
you had come into our irc room months ago and xommunicated in real time I
guarantee this bug would be in a different state.

Regardless of your personal opinions about Bugzilla, it's where we file bugs about
our software--all of it. It's completely reasonable to expect people to file bugs here, and they should be treated with just as much interest and care as things tracked in Mingle, Trello, or reported to the IRC ether.

(In reply to comment #15)

(In reply to comment #14)

Bugzilla is how one files bugs against a product--sorry you don't like it.
The
nice thing is it's asynchronous so if someone is in a different timezone they
can easily report issues without waiting for you to be around :)

Bugzilla is an excellent way to track bugs, but for solving problems nothing
beats communicating with a human being in real time. The foundation uses
real
time communication within teams extensively to organize and ensure the work
they want to get done happens. Embrace the most effective forms of
communication you can.

Lucky for those in other timezones, we have team members in 2 american
timezones, europe and australia.

The point is simply: there was a bug report by a respected member of the platform team (whether or not you knew he was the point person for the API); please respond to bug reports. People are busy and reviewing a lot of people's code and can't be on point to track everyone down all the time (seriously, if Platform had to do that, we'd never get anything else done).

Please take some initiative and review your bugzilla tickets and ask for clarification where needed.

(Off-topic, like the rest of the last 6 or so comments, but, I wanted to publicly say: Sorry for my tone. We'll work together on addressing this bug.)

(In reply to comment #9)

Possibly. There are some community members who would find a way to take
offence to "how is your day going?". I don't particularly think we should base
our decisions around pandering to them.

This is unacceptable attitude, Oliver. You in particular should know better than that.

(In reply to comment #13)

Personally I find impersonal communication methods like bugzilla a passive
aggressive way to air issues without actually talking to anyone directly.

I'm severely confused. After reading comment 0 out of the blue, I expected comment 1 to start like this: "Great Brad, thanks for your thorough review of our API module! It's very useful when someone helps us learn more about/do better with some specialised areas of the code rather than let us alone hitting against a wall". And then a conversation to clarify the various actionable items and to log them properly in atomic bugs or cards.

(In reply to comment #19)

(In reply to comment #9)

Possibly. There are some community members who would find a way to take
offence to "how is your day going?". I don't particularly think we should base
our decisions around pandering to them.

This is unacceptable attitude, Oliver. You in particular should know better
than that.

Sorry, I should be clearer; the 'them' is 'the sort of people who would find a way to take offence...'

So, to expand: I fully believe that the community should be factored into the decision-making process, directly and indirectly - but factored in is very different from being a trump card. If the argument is "there will be people who look at the API and think it's Us Doing The VisualEditor All Over Again", that's a bad argument, and the sort of people who would jump from 'the minimum viable product is not perfect' to 'everything is going to be terrible forever' are not people I am particularly interested in basing our decisions on, because anyone who _makes_ that cognitive leap is clearly not approaching things rationally, and so it becomes very hard to actually compromise with them - if compromising with them is actually worthwhile in the first place.

So, back on topic...

From Brian on bug 60178 comment 28:
"I would think the appropriate thing to do (If we must do this) is to include a note in the auto-generated api documentation."

Can this be done, or explain why not? I still think a note to BAG or similar is good in addition, personally.

(In reply to comment #22)

So, back on topic...

From Brian on bug 60178 comment 28:
"I would think the appropriate thing to do (If we must do this) is to
include a
note in the auto-generated api documentation."

Can this be done

Easily. Just include the necessary text in the string returned from the module's getDescription() method.

(In reply to comment #23)

Easily. Just include the necessary text in the string returned from the
module's getDescription() method.

Sorry, more meant "Can the team please do this soon, or say why not?" :)

(In reply to comment #24)

(In reply to comment #23)

Easily. Just include the necessary text in the string returned from the
module's getDescription() method.

Sorry, more meant "Can the team please do this soon, or say why not?" :)

https://gerrit.wikimedia.org/r/#/c/110967/ should address that.

(In reply to comment #21)

So, to expand: I fully believe that the community should be factored into the
decision-making process, directly and indirectly - but factored in is very
different from being a trump card. If the argument is "there will be people
who
look at the API and think it's Us Doing The VisualEditor All Over Again",

I'm looking at this and thinking, "Oh, no, it's them doing WikiEditor all over again," where a product is deployed to wikis before the API is properly documented or becomes stable, and bot/script developers practically have to reverse engineer it in order to update all of their code in time.

I mean, you're at the point of field testing it in production, but there apparently isn't even a specification for how the real API will work, let alone a place where the people who will be using it might actually be able to make some input.

(In reply to comment #21)

Alex Z., what do you and others want from a Flow API? How can we best collaborate on it? A Flow board makes API requests for all in-page updates, so Flow heavily exercises its API while we implement Brad Jorsch's feedback, but I'm concerned about what we overlook. I filed bug 60808 and bug 60809, but I'm just guessing.

Erik Bernhardson posted https://en.wikipedia.org/wiki/Wikipedia:Bot_owners%27_noticeboard#New_extension:_Flow , so that's another venue. I mentioned these bugs there.

I've created https://www.mediawiki.org/wiki/Flow/Architecture/API with links to these, and empty sections awaiting use cases. I'll try to keep it updated.

Thanks everyone.

This bug is marked as "PATCH_TO_REVIEW", but the patch in question is marked as work in progress ("[WIP]"). For clarity, I'm moving this back out to "assigned", since I understand that Kunal is working on it.

Change 107411 merged by jenkins-bot:
API: Revamp action=flow

https://gerrit.wikimedia.org/r/107411

action=flow looks much better!

list=flow still needs work though, so reopening.

(In reply to Brad Jorsch from comment #0)

For list=flow:

  • The link returned by getHelpUrls() does not actually document this API

module.

I had removed this earlier, but I'll make sure to add it in once the parameters are finalized.

  • The "flowaction" parameter does not list available actions.

Removed in I73347cb3b0a8cf35881691f50e70be10d10571b8 because it's pointless.

  • Same criticisms of the "params" blob as above.

After looking through all the JS that call this, they're either options to control the result format, like "contentFormat": "wikitext", or they're a post-id. I'll turn this into 2-3 more parameters to the API module directly.

  • You are setting "_element" directly instead of using ApiResult's methods.

Note to self that this needs to be fixed in all implementations of Block::renderAPI as well.

  • You should be checking the return from ApiResult's addValue method and

properly handling failure.

From the documentation, addValue will return false if the data is too big. We'll need some kind of continuation parameter...I'm not exactly sure how Flow handles that internally.

https://gerrit.wikimedia.org/r/#/q/project:mediawiki/extensions/Flow+topic:flow-api,n,z

Marking as fixed, the issues in comment 0 were fixed, and it supports autogenerated documentation.

What's still missing is the on-wiki documentation, that's bug 58361.