Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

swarming: Despaghettify bot_config hook calls. #271

Open
vadimsht opened this issue Jul 19, 2016 · 1 comment
Open

swarming: Despaghettify bot_config hook calls. #271

vadimsht opened this issue Jul 19, 2016 · 1 comment

Comments

@vadimsht
Copy link
Contributor

vadimsht commented Jul 19, 2016

Bot startup process is messy.

I'll be taking specifically about Chrome infra bot_config.py case, as a primary example.

Here's how it works:

  • bot_config.get_dimensions and bot_config.get_state are called with bot.Bot object that has id == 'none' (and the rest of the properties missing). bot_config.get_dimensions is actually using android.get_devices, but Android subsystem hasn't been initialized yet (because it is initialized in on_bot_startup), and so initial dimensions and state are bogus.
  • Then on_bot_startup is called. It assumes bot.Bot object already has dimensions set (which are set to possibly bogus values, since Android subsystem hasn't been initialized yet). It initializes Android subsystem.
  • bot_config.get_dimensions and bot_config.get_state are called again. Now with correct bot.Bot object and correct Android subsystem state.
  • Finally /handshake endpoint is called, it receives correct dimensions and state.

This is at least very confusing (bordering on "easy to break the world", e.g. https://chromereviews.googleplex.com/465757013).

I'm now trying to return additional data from /handshake, that can be used in get_dimensions to make decisions. Unfortunately, get_dimensions is called twice in different circumstances even before /handshake is called. Adding third flavor of environment in which get_dimensions is called is too much.


Proposal:

  1. Postulate that on_bot_startup is called before any other hook (including before get_dimensions).
  2. Document that on_bot_startup receives special uninitialized bot.Bot object.
  3. Teach on_bot_startup to return bot ID, initial set of dimensions and state, send them to the /handshake.
  4. Document that /handshake receives initial dimensions and state that may be changed in the future by get_dimensions and get_state.
  5. Document that get_dimensions and get_state are called immediately before /poll (e.g. after all initialization and initial handshake).
  6. Forbid ID dimension to be changed by get_dimensions (bot changing its ID midlife doesn't make much sense).

The new flow then would be:

  • on_bot_startup is called (with bot.Bot.id == 'none'), it initializes Android subsystem and bot ID and initial dimensions and state.
  • /handshake is called. It may return additional server-supplied data, fetched based on bot's ID.
  • get_dimensions and get_state are called. They may use state set up in on_bot_startup and data returned by /handshake.
  • /poll
@maruel
Copy link
Member

maruel commented Jul 19, 2016

About 2., maybe not pass a Bot object?

lgtm, thanks for looking at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants
@maruel @vadimsht and others