Closed Bug 674316 Opened 14 years ago Closed 14 years ago

Account portal stage bug creating a user - lib handles integer badly

Categories

(Cloud Services :: Server: Core, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: telliott, Assigned: telliott)

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

Giving a value of 1 to accountStatus bombs out, even though it's defined in the schema as an int. making it '1' solves the problem. Also cleaning up a couple small errors that we found while on stage.
Attachment #548546 - Flags: review?
Attachment #548546 - Flags: review? → review?(rmiller)
Added a little bit I didn't mean to. Pulling that out
Assignee: nobody → telliott
Attachment #548546 - Attachment is obsolete: true
Attachment #548546 - Flags: review?(rmiller)
Attachment #548549 - Flags: review?(rmiller)
Comment on attachment 548549 [details] [diff] [review] revised version to clean up the problems Review of attachment 548549 [details] [diff] [review]: -----------------------------------------------------------------
Attachment #548549 - Flags: review?(rmiller) → review+
1/ there's an API in the ldap lib to do this in fact. It's not used right now but we should use it. It converts all values to strings for you 2/ if it bombs out, are we actually catching that error properly ? 3/ why are you adding **kw in this patch ?
(In reply to comment #3) > 1/ there's an API in the ldap lib to do this in fact. It's not used right > now but we should use it. It converts all values to strings for you > OK, we can do that in the future. > 2/ if it bombs out, are we actually catching that error properly ? > yes. The problem is that the test wasn't running properly before and I hadn't realized it. Once I got it to actually test this, it fails. > 3/ why are you adding **kw in this patch ? It's another bugfix I caught when I got the tests to run. Which I'm pretty sure that we discussed and approved before elsewhere and can dig out once I get this to atoll.
(In reply to comment #4) > OK, we can do that in the future. Why should we postpone a better future-proof fix ? > > > 2/ if it bombs out, are we actually catching that error properly ? > > > > yes. The problem is that the test wasn't running properly before and I > hadn't realized it. Once I got it to actually test this, it fails. > > > 3/ why are you adding **kw in this patch ? > > It's another bugfix I caught when I got the tests to run. Which I'm pretty > sure that we discussed and approved before elsewhere and can dig out once I > get this to atoll. This sounds like another issue to me
(In reply to comment #6) > (In reply to comment #4) > > OK, we can do that in the future. > > Why should we postpone a better future-proof fix ? > We shouldn't. We'll do it, possibly very quickly. But the current solution does work. > > > 3/ why are you adding **kw in this patch ? > > > > It's another bugfix I caught when I got the tests to run. Which I'm pretty > > sure that we discussed and approved before elsewhere and can dig out once I > > get this to atoll. > > This sounds like another issue to me You are correct. Mea culpa, I was trying to get this one out asap. I'll file a separate bug for that issue.
(In reply to comment #7) > > This sounds like another issue to me > > You are correct. Mea culpa, I was trying to get this one out asap. I'll file > a separate bug for that issue. Filed Bug 674316 to track this one
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
(In reply to comment #8) > Filed Bug 674316 to track this one Oops. That's Bug 674347 Also filed Bug 674414 for the ldap modlist issue.
(In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #4) > > > OK, we can do that in the future. > > > > Why should we postpone a better future-proof fix ? > > > > We shouldn't. We'll do it, possibly very quickly. But the current solution > does work. It works, but that does not explain why you did not apply the better solution right away instead of duplicating the workload, since it was not a hotfix.
Status: RESOLVED → VERIFIED
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: