Reviewing the customer create path

[Bryan Kearney]

I took my own advice.. And reviewed one of the existing stories. I have 
a bunch of comments below.

Reviewing the Create Consumer Code
==================================

ConsumerResource
----------------
- I assume the owner creation code in the ctor should be a fixme.
- Should the Identity cert creation go into the Curator? This seems like 
business logic.
- In general, many of these classes actually implement business logic 
(much of the bind logic). Should this move into a lower level business 
curator? Perhaps there are other interim curators like the entitler. 
Although the binder is probably a bad name :)
- Any reason to not rip out the commented out code blocks for the old 
methods?

IdentityCertAdapter
-------------------
- Currently, the adapter needs to (1) Generate and (2) Persist the new 
certficate in the IdentityCertificate database. Do we want the candlepin 
engine proper to actually do the persisting? It seems like this is a 
little bit too much bleed over across the API.

Default Identity Certificate
----------------------------
- nextSerialNumber has a bit of a race condition :)
- why is username in the DN?

-- bk

[Devan Goodwin]

On Fri, Mar 26, 2010 at 4:07 PM, Bryan Kearney <bkearney(a)redhat.com&gt; wrote:
>  I took my own advice.. And reviewed one of the existing stories. I
> have
>  a bunch of comments below.
>
>  Reviewing the Create Consumer Code
>  ==================================
>
>  ConsumerResource
>  ----------------
>  - I assume the owner creation code in the ctor should be a fixme. 
+1

>  - Should the Identity cert creation go into the Curator? This seems
> like
>  business logic. 
+1, may as well move it into ConsumerCurator.create.

>  - In general, many of these classes actually implement business
> logic
>  (much of the bind logic). Should this move into a lower level business
>  curator? Perhaps there are other interim curators like the entitler. 
+1 yeah I'd say so.

>  Although the binder is probably a bad name :) 
LOL

>  - Any reason to not rip out the commented out code blocks for the
> old
>  methods? 
Not that I know of.

>  IdentityCertAdapter
>  -------------------
>  - Currently, the adapter needs to (1) Generate and (2) Persist the new
>  certficate in the IdentityCertificate database. Do we want the candlepin
>  engine proper to actually do the persisting? It seems like this is a
>  little bit too much bleed over across the API. 
Seems like we've got a confused state here, the cert service adapter
is responsible for generating the cert, is it also responsible for
storage? I guess we're going the opposite direction, generate only,
Candlepin itself will do the storage and retrieval. This will require
some small changes to both the identity cert adapters and entitlement
cert adapters.

>  Default Identity Certificate
>  ----------------------------
>  - nextSerialNumber has a bit of a race condition :) 
Yes I was planning to get the serial number generation fixed up for
both types of certs, need to add a task for this but have to wait
until next sprint.

>  - why is username in the DN? 
Not sure on this one.

Devan

-- 
Devan Goodwin <dgoodwin(a)rm-rf.ca&gt;
http://rm-rf.ca

[Justin Harris]

----- "Bryan Kearney" <bkearney(a)redhat.com&gt; wrote:

>  I took my own advice.. And reviewed one of the existing stories. I
>  have 
>  a bunch of comments below.
>  
>  Reviewing the Create Consumer Code
>  ==================================
>  
>  ConsumerResource
>  ----------------
>  - I assume the owner creation code in the ctor should be a fixme.
>  - Should the Identity cert creation go into the Curator? This seems
>  like 
>  business logic.
>  - In general, many of these classes actually implement business logic
>  
>  (much of the bind logic). Should this move into a lower level business
>  
>  curator? Perhaps there are other interim curators like the entitler. 
>  Although the binder is probably a bad name :)
>  - Any reason to not rip out the commented out code blocks for the old
>  
>  methods?
>  
>  IdentityCertAdapter
>  -------------------
>  - Currently, the adapter needs to (1) Generate and (2) Persist the new
>  
>  certficate in the IdentityCertificate database. Do we want the
>  candlepin 
>  engine proper to actually do the persisting? It seems like this is a 
>  little bit too much bleed over across the API. 
Yeah, I would think so.  Is this consistent with how the entitlement certs are
being created/persisted?  I would think that we want have similar behaviors.


>  
>  Default Identity Certificate
>  ----------------------------
>  - nextSerialNumber has a bit of a race condition :)
>  - why is username in the DN? 
This was a request from Pradeep to include it.  I didn't really
ask any questions.  Should we make them issue a separate call to
get this?

>  
>  -- bk
>  _______________________________________________
>  candlepin mailing list
>  candlepin(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/candlepin

[Jesus M. Rodriguez]

On Fri, Mar 26, 2010 at 3:07 PM, Bryan Kearney <bkearney(a)redhat.com&gt; wrote:
[snip]

>  Although the binder is probably a bad name :) 
I think binder is a rockin name.

>  - Any reason to not rip out the commented out code blocks for the
> old
>  methods? 
removed them


jesus

[jesus rodriguez]

On Fri, Mar 26, 2010 at 03:07:42PM -0400, Bryan Kearney wrote:
>  I took my own advice.. And reviewed one of the existing stories. I
> have 
>  a bunch of comments below.
>  
>  Reviewing the Create Consumer Code
>  ==================================
>  
>  ConsumerResource
>  ----------------
>  - I assume the owner creation code in the ctor should be a fixme. 
+1

>  - Should the Identity cert creation go into the Curator? This seems
> like 
>  business logic. 
Probably not, I just reworked the create method, but I still left it
mostly intact. I could probably use even more cleanup like you mention.

>  - In general, many of these classes actually implement business logic
>
>  (much of the bind logic). Should this move into a lower level business 
>  curator? Perhaps there are other interim curators like the entitler. 
>  Although the binder is probably a bad name :) 
+1 to binder

>  - Any reason to not rip out the commented out code blocks for the old
>
>  methods? 
like I said in a previous response, I ripped these out.

jesus

-- 
jesus m. rodriguez          | jesusr(a)redhat.com
principal software engineer | irc: zeus
red hat systems management  | 919.754.4413 (w)
rhce # 805008586930012      | 919.623.0080 (c)
+---------------------------------------------+
|   "Those who cannot remember the past       |
|    are condemned to repeat it."             |
|                        -- George Santayana  |
+---------------------------------------------+

[Bryan Kearney]

On 03/26/2010 04:19 PM, Jesus Rodriguez wrote:
>  On Fri, Mar 26, 2010 at 03:07:42PM -0400, Bryan Kearney wrote:
> > I took my own advice.. And reviewed one of the existing stories. I have
> > a bunch of comments below.
> >
> > Reviewing the Create Consumer Code
> > ==================================
> >
> > ConsumerResource
> > ----------------
> > - I assume the owner creation code in the ctor should be a fixme.
>
>  +1
>
> > - Should the Identity cert creation go into the Curator? This seems like
> > business logic.
>
>  Probably not, I just reworked the create method, but I still left it
>  mostly intact. I could probably use even more cleanup like you mention.
>  
Why not? I am just curious on this one.. trying to figure out the role 
of the curator.

-- bk

[jesus rodriguez]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/29/2010 10:42 AM, Bryan Kearney wrote:
>  On 03/26/2010 04:19 PM, Jesus Rodriguez wrote:
> > On Fri, Mar 26, 2010 at 03:07:42PM -0400, Bryan Kearney wrote:
> >> I took my own advice.. And reviewed one of the existing stories. I have
> >> a bunch of comments below.
> >>
> >> Reviewing the Create Consumer Code
> >> ==================================
> >>
> >> ConsumerResource
> >> ----------------
> >> - I assume the owner creation code in the ctor should be a fixme.
> >
> > +1
> >
> >> - Should the Identity cert creation go into the Curator? This seems like
> >> business logic.
> >
> > Probably not, I just reworked the create method, but I still left it
> > mostly intact. I could probably use even more cleanup like you mention.
> >
>  
>  Why not? I am just curious on this one.. trying to figure out the role 
>  of the curator. 
Cuz that sentence didn't make any sense :) I *think* I was trying to
say that it probably was not moved to the curator. I agree it probably
SHOULD go there :)

sorry for the confusion.

jesus

- -- 
jesus m. rodriguez          | jesusr(a)redhat.com
principal software engineer | irc: zeus
red hat systems management  | 919.754.4413 (w)
rhce # 805008586930012      | 919.623.0080 (c)
+---------------------------------------------+
|   "Those who cannot remember the past       |
|    are condemned to repeat it."             |
|                        -- George Santayana  |
+---------------------------------------------+

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iEYEARECAAYFAkuw1N8ACgkQvJZ57YntiYOP9QCggiU1E/TgdtZKh6QV5UnfWsuz
> 9wcAoL+/hfQx9yHRwpNvUXMmrSlqBcOp
> =J8qZ
> -----END PGP SIGNATURE-----