defaulting to staging is confusing

[Martin Pool]

launchpadlib currently defaults to talking to the staging database server, which means that it looks to developers like they have the real thing, but it's not quite up to date, and their changes won't take effect in what they think of as Launchpad itself.

This is probably being done so as to encourage developers to try out their app on staging before using it for real, which is a good intention, but there are some problems:

 * It's not very obvious to new developers why things don't work: lplib by default just "silently discards" the changes you make.
 * staging is not necessarily the same as development.
 * It requires developers to know about and code for Launchpad's deployment practices, which should not be their concern, and it requires updates when they change. (Basically every lplib app now needs to be updated away from using edge.)
 * It now comes up in a default firing-blanks mode by using anonymous-readonly access.

This is described in <https://help.launchpad.net/API/launchpadlib#Anonymous%20access> but that's a long page and the impact is not obvious.

[Leonard Richardson (leonardr)]

How about logging an info message when a Launchpad object is created against staging? That would continue to prevent against accidents while making it obvious to any developer what is going on.

[Leonard Richardson (leonardr)]

Also, production would be a better default for anonymous access, since there's no danger of accidents.

[Martin Pool (mbp)]

I think it's better to just default to lpnet. Otherwise the first thing people will do is just update it to use that, so I doubt this really gets any protection.

[Leonard Richardson (leonardr)]

Here's the email I sent to launchpad-users about this:
---

Right now, if you run this code:

>>> from launchpadlib.launchpad import Launchpad
>>> l = Launchpad.login_with("my app")
>>> print l.bugs[714043].description

It'll run against staging.launchpad.net. To run against the production server you'll need to pass service_root="production" into login_with(). This causes confusion if you don't know what's going on--your changes to the production server are reverted, and the data is slightly out of date.

In bug 714043 (https://bugs.launchpad.net/launchpadlib/+bug/714043), Martin Pool suggested changing the default to production.

I wrote a branch (https://code.launchpad.net/~leonardr/launchpadlib/bug-714043/+merge/48842) which preserves staging as the default, but prints a warning whenever the default is used. My branch also makes production the default for *anonymous* access. Anonymous access is read-only, so you can't break anything. You might as well go against production and get the most recent data.

Martin prefers his solution and I prefer mine. Since it comes down to what the majority of developers would prefer, I'd like to get feedback from the community at large. If you have a strong opinion one way or the other, please leave a comment in bug 714043.

Summary of my position: one purpose of the staging server is to let users try things out without ruining their data. Developing a launchpadlib script is exactly the kind of thing you'd want to do on a staging server, since a bug in your script can ruin your data. The warning may be annoying to experienced developers, but it's useful to inexperienced developers, and it gives you a reminder that you'll need to switch to production for your changes to take effect in the real world.

I'll let Martin summarize his own position, but here's my stab at it:

1. Developers in general want to work against production, and printing this warning will just remind them to change it, which they will do immediately.
2. Making production the default everywhere maintains consistency between anonymous and authenticated access.
3. Other web service providers that publish client libraries don't have the client library go against a staging server by default.

Again, please leave your comments in bug 714043 so we have a unified record of the discussion: https://bugs.launchpad.net/launchpadlib/+bug/714043

Leonard

[Brian Murray (brian-murray)]

Personally, I've also been nervous about what someone could do with API. Having said that we haven't had any problems yet so if it wouldn't be too hard to clean up a lot of changes made via the API then defaulting to production seems the most convenient for everyone.

[Vincent Ladeuil (vila)]

The bit that annoys me is that anonymous access is special-cased. I'd prefer the same behavior whether the access is anonymous or not.

An alternative option would be to force the dev to specify whether he wants production or staging (which may give *him* a hint that staging may be a good option for some --dry-run mode).

[Martin Pool (mbp)]

On 9 February 2011 05:25, Vincent Ladeuil <email address hidden> wrote:
> The bit that annoys me is that anonymous access is special-cased. I'd
> prefer the same behavior whether the access is anonymous or not.
>
> An alternative option would be to force the dev to specify whether he
> wants production or staging (which may give *him* a hint that staging
> may be a good option for some --dry-run mode).

I can certainly imagine app developers wanting a dry run mode; indeed
there are bugs asking for it to be done in a better way than just
through staging.

1- I'm not so convinced it should be on by default; most interfaces
default to 'actually do stuff' and then have a '--dry-run' option.
Just look at all the unix commands that have exactly that option,
whereas none that I can recall have an --actually-do-it.

2- Expressing 'dry_run', whatever the default, by choosing a service
root url, is an abstraction inversion. If Launchpad later decides it
wants to, for instance, provide a web service on the production
instance that discards data, or to do away with the staging cluster,
or whatever, every lplib-using app will need to be updated.
Conversely, if lplib provides a 'dry_run' option then we would just
need to update lplib.

3- Having most apps explicitly give a URL means that lplib can't tell
whether the app actually has an opinion about which instance to use,
or if it just wants things to work. Knowing the difference could be
useful: for instance we could have an environment variable that lets
users change it without needing to build this into every single app;
or lplib could (hypothetically) choose a good URL at run time based on
the user's location; ...

[Martin Pool (mbp)]

A couple of other things:

 * Many clients are readonly (eg https://launchpad.net/kanban), so they may not notice they're using staging, but they will get out-of-date data and they will likely get poorer service (since staging is more often offline.) A warning would handle this, assuming people see it, which is not certain.

 * There's already a way for people to ask for a readonly access token or anonymous access if they don't want to take the risk of changing things.

[Julian Edwards (julian-edwards)]

Why do we need a default at all? What's wrong with forcing the user to explicitly specify which system they want to access?

[Leonard Richardson (leonardr)]

"What's wrong with forcing the user to explicitly specify which system they want to access?"

I thought this would break backwards compatibility, but upon further reflection it would only break code that has been running against staging this whole time. Such code is already broken, though for read-only scripts there's a difference between "my script gets data that's slightly out of date" and "my script crashes with an exception".

Given the diversity of opinion, this may be the best solution.

[Leonard Richardson (leonardr)]

Besides which, we just broke launchpadlib for anyone who was using edge.

[Joey Stanford (joey)]

In all of my api scripts I manually specify the system. If we needed something as a default then I'd prefer staging since tests and trials won't affect lpnet. The one thing, from a nicety standpoint, that I wish we had was a little automated output that indicates which service root you are attached to when you run a program.

[Martin Pool (mbp)]

On 10 February 2011 01:30, Leonard Richardson
<email address hidden> wrote:
> Besides which, we just broke launchpadlib for anyone who was using edge.

Just this reason. I'd rather not go through this again if we decide
later that staging isn't the best sandbox for users to test stuff.

[Martin Pool (mbp)]

On 9 February 2011 22:41, Julian Edwards <email address hidden> wrote:
> Why do we need a default at all?  What's wrong with forcing the user to
> explicitly specify which system they want to access?

I don't mind making them specify it explicitly as long as that's done
at the right level of abstraction: "I want to actually change things
in Launchpad", or "I want a sandbox where I can test my app without
breaking things" or maybe something else...

At the moment in Ubuntu's stable release, they must either hardcode a
URL or do the moral equivalent through string manipulation, because
there is no symbol for LPNET_SERVICE_ROOT. And that's just to get the
result of what almost every other webservice client library does,
which is to get a real connection to the main instance. Most client
users or authors don't care about lp's deployment scheme, and it
changes faster than clients are updated.

There is a lot of duplication of this across lp clients.

[Robert Collins (lifeless)]

> Just this reason.  I'd rather not go through this again if we decide
> later that staging isn't the best sandbox for users to test stuff.

Staging certainly isn't the best sandbox - it runs the next schema,
not the live one. Qastaging is the sandbox we link to from the
website.

OTOH I don't think it makes huge difference what sandbox.

+1 on hiding the implementation detail.

[Leonard Richardson (leonardr)]

"At the moment in Ubuntu's stable release, they must either hardcode a
URL or do the moral equivalent through string manipulation, because
there is no symbol for LPNET_SERVICE_ROOT"

In 1.9.5, which is about to go into Ubuntu, the symbol for LPNET_SERVICE_ROOT is the string 'production'. We have similar simple-string aliases for all the active servers. I can add aliases for higher levels of abstraction as well, but I would like to know which aliases people will actually use.

[Leonard Richardson (leonardr)]

Actually, even in 1.6.x, the symbol for LPNET_SERVICE_ROOT is the string 'production'.

[Leonard Richardson (leonardr)]

Unless anyone objects, I'm going to remove the default value for service_root altogether, in the first post-Natty version of launchpadlib.

[Martin Pool (mbp)]

On 15 February 2011 02:07, Leonard Richardson
<email address hidden> wrote:
> "At the moment in Ubuntu's stable release, they must either hardcode a
> URL or do the moral equivalent through string manipulation, because
> there is no symbol for LPNET_SERVICE_ROOT"
>
> In 1.9.5, which is about to go into Ubuntu, the symbol for
> LPNET_SERVICE_ROOT is the string 'production'. We have similar simple-
> string aliases for all the active servers. I can add aliases for higher
> levels of abstraction as well, but I would like to know which aliases
> people will actually use.

Are you really changing that parameter from being a URL to being a
symbolic name? That seems likely to break client applications.

[Leonard Richardson (leonardr)]

"Are you really changing that parameter from being a URL to being a
symbolic name? That seems likely to break client applications."

I didn't explain this very well. Let me try again.

1. The value of uris.LPNET_SERVICE_ROOT is 'https://api.launchpad.net/api/'. This has been the case since before 1.6.1. You can run this code and it will work, in maverick or natty.
from launchpadlib.launchpad import Launchpad
from launchpadlib.uris import LPNET_SERVICE_ROOT
Launchpad.login_with("foo", LPNET_SERVICE_ROOT)

If this doesn't work, I'd like to know why.

2. However, I never envisioned this usage. This is the usage I envisioned:

from launchpadlib.launchpad import Launchpad
Launchpad.login_with("foo", "production")

This will also work in maverick or natty, and if it doesn't work I'd like to know why.

3. Behind the scenes, the string "production" is mapped to the string 'https://api.launchpad.net/api/'.

4. If I were to change the constant LPNET_SERVICE_ROOT so that it was 'production' instead of 'https://api.launchpad.net/api/', this should have no effect on clients. Unless, some client is assuming that LPNET_SERVICE_ROOT is a URL and doing URL manipulation on the value.

5. I don't have a problem with someone prefering to import a Python constant LPNET_SERVICE_ROOT rather than use a hard-coded string "production". But if you're all about importing constants, you shouldn't also be performing URL manipulation on the constants. It shouldn't make a difference to you if I change the value of the constants.

So, coming back to your original statement:

"At the moment in Ubuntu's stable release, they must either hardcode a
URL or do the moral equivalent through string manipulation, because
there is no symbol for LPNET_SERVICE_ROOT"

In Ubuntu's stable release, there is a symbol for LPNET_SERVICE_ROOT: it's uris.LPNET_SERVICE_ROOT. You can also use the string "production". If it turns out people would rather use uris.LPNET_SERVICE_ROOT than "production", I would like to change the value of uris.LPNET_SERVICE_ROOT to "production". Otherwise, people might get the idea that they can do URL manipulation on these constants, when they're actually supposed to be nothing but useful constants for people to pass into login_with().

[William Grant (wgrant)]

On 15/02/11 10:53, Leonard Richardson wrote:
> "Are you really changing that parameter from being a URL to being a
> symbolic name? That seems likely to break client applications."
>
> I didn't explain this very well. Let me try again.
>
> 1. The value of uris.LPNET_SERVICE_ROOT is 'https://api.launchpad.net/api/'. This has been the case since before 1.6.1. You can run this code and it will work, in maverick or natty.

I presume the trailing 'api/' is a thinko? I thought that was only
present on the webapp domains.

> [snip]
> "At the moment in Ubuntu's stable release, they must either hardcode a
> URL or do the moral equivalent through string manipulation, because
> there is no symbol for LPNET_SERVICE_ROOT"
>
> In Ubuntu's stable release, there is a symbol for LPNET_SERVICE_ROOT:
> it's uris.LPNET_SERVICE_ROOT. You can also use the string "production".
> If it turns out people would rather use uris.LPNET_SERVICE_ROOT than
> "production", I would like to change the value of
> uris.LPNET_SERVICE_ROOT to "production". Otherwise, people might get the
> idea that they can do URL manipulation on these constants, when they're
> actually supposed to be nothing but useful constants for people to pass
> into login_with().

LPNET_SERVICE_ROOT is not in launchpadlib.launchpad, where
EDGE_SERVICE_ROOT is (was) for compatibility reasons. I don't think many
people thought to look in launchpadlib.uris.

And people do perform URL manipulations on it. Mostly to emulate
web_url, I suspect.

[Martin Pool (mbp)]

We seem to be getting a bit beyond the scope of this particular bug.

I'm a bit alarmed by what seem to be unnecessary client-breaking API
changes in launchpadlib. I'm all for breaking APIs if there's a good
reason, but I don't understand the reasons for these changes at the
moment.

Client applications do rely on these things being URLs, either because
they are trying to work around lplib bugs or lacunae (eg having no
constant for LPNET, having no web_url property), or because they just
simply want the actual URL to access in other ways. For instance
txrestfulclient and wrested don't use Launchpadlib to do the actual
http calls but they do (or could) use these constants rather than
hardcoding the URLs themselves.

See for instance
<https://code.launchpad.net/~maxb/bzr/launchpadlib-service-root-api-compat/+merge/47885>

If you want to add symbolic constants for the tokens 'production' etc,
by all means do. That is very much in the spirit of getting clients
away from specifying URLs when they don't really care about the
particular URL. However please add a new symbol for that rather than
repurposing an existing well defined symbol.

> 4. If I were to change the constant LPNET_SERVICE_ROOT so that it was
> 'production' instead of 'https://api.launchpad.net/api/', this should
> have no effect on clients. Unless, some client is assuming that
> LPNET_SERVICE_ROOT is a URL and doing URL manipulation on the value.

They do assume it's a URL. It's always been a URL, and the term
'service root' is I think generally understood to mean a URL.

> So, coming back to your original statement:
>
> "At the moment in Ubuntu's stable release, they must either hardcode a
> URL or do the moral equivalent through string manipulation, because
> there is no symbol for LPNET_SERVICE_ROOT"
>
> In Ubuntu's stable release, there is a symbol for LPNET_SERVICE_ROOT:
> it's uris.LPNET_SERVICE_ROOT. You can also use the string "production".
> If it turns out people would rather use uris.LPNET_SERVICE_ROOT than
> "production", I would like to change the value of
> uris.LPNET_SERVICE_ROOT to "production".

Sorry, I was confused by the move from launchpadlib.launchpad.* to
launchpadlib.uris.*

> Otherwise, people might get the
> idea that they can do URL manipulation on these constants, when they're
> actually supposed to be nothing but useful constants for people to pass
> into login_with().

Why does it matter?

[Martin Pool (mbp)]

OK, I think I now understand what Leonard's saying.

He's not actually proposing to change the meaning/binding of LPNET_SERVICE_ROOT. (I think that would be a bad idea and likely to break client applications. Bug 719144 argues against this, but it looks like the change is not actually landed.)

Rather, for clients targeting Maverick's launchpadlib or later, he wants them to specify say 'production' or 'staging' rather than a specific URL. That's ok with me.