[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Pulp-list] Minor usability problem with stale cached login credentials



On 03/22/2012 09:03 PM, Nick Coghlan wrote:
On 03/23/2012 04:20 AM, Jay Dobies wrote:
This is one of those things that annoyed us in the beginning and after a
while of never getting around to fixing it, we developed blinders and
don't even notice anymore :)

This sprint I'm looking to get login/logout functionality into the v2
client, I'll make sure I don't fall into the same trap again.

If you're replacing the client then I won't worry about raising the bug.
While I'll be using my custom client (which hooks into the v1 client
authentication code) for PulpDist 0.1.0, the new pulp-admin integration
hooks are definitely something I want to look at for 0.2.0.

If you're curious about how my custom Pulp v2 API client currently
works, the docs are here:

http://readthedocs.org/docs/pulpdist/en/latest/cli.html

Nice docs  :)

It looks like you've reduced the create/add importer stuff to a single step in init. I'm doing the same thing for the RPM client and ran into a problem whose solution you might be interested in.

The create repo call is pretty easy to make succeed, there's very little that can go wrong. The add importer call, on the other hand, has a lot of potential to fail due to invalid config. When that happens, I was left with a created repo but no importer on it and no desire to expose to the user where that distinction lies.

My first thought was to have the client delete the newly created repo in that case. That has a problem too. With the addition of the coordinator, it's possible that delete call won't immediately execute and return a 200, but rather it will queue up the operation a 202 accepted is returned.

That's due to our operation conflict resolution, which we've been calling our "coordinator" layer. For example, if there's a sync currently executing against the repo, we can't immediately delete it, so the delete is queued up after it with no assumptions on when it will run.

That behavior makes sense, but it makes that CLI rollback wonky. I can't automatically delete the created repo if the add importer fails because it may be postponed. It obviously won't be due to a sync, but if the tasking queue is currently busy. Ideally that shouldn't happen once we tweak the creative math to hopefully minimize that.

I've always had in the back of my head that the three call process for creating a repo and adding an importer and a distributor might be problematic. So in response to hitting this, I added a single aggregate call on the server that will accept importer/distributor information in the create call. That way the server can manage the transactioning and there's no chance that clean up call for a failed importer/distributor will be delayed.

It's the same POST call that create is now, but has the following optional fields:
- importer_type_id - if specified, the call will attempt to add an importer
- importer_config - same config you'd pass to the add importer REST API
- distributors - list of distributor tuples; if specified each item in the list will be added as a distributor (unspecified = None or empty list in this case). The tuple values are, in order (again, these correspond to the same types and restrictions as the add distributor REST API):
  - distributor type id
  - distributor config
  - auto publish flag
  - distributor id

That greatly simplified the CLI handling of create repo for me. Let me know if it's useful and what you think.

And the main() implementation is here:

git.fedorahosted.org/git/?p=pulpdist.git;a=blob;f=src/pulpdist/cli/repo_cli.py


Cheers,
Nick.


--
Jay Dobies
Freenode: jdob @ #pulp
http://pulpproject.org | http://blog.pulpproject.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]