[SUSTAIN-632] Do not update the users table to contain the public key. The keys table should be the only source of truth #1383

PrajaktaPurohit · 2017-08-24T00:35:46Z

TODO:

Add sqitch tag
Add omnitruck changes
If knife create client is the only way to create clients, this bug does not affect clients.
Update function returns [[<<"update_user">>, <<>>]] that does not get handled in our code

==> /var/log/opscode/opscode-erchef/current <==
2017-08-23_22:48:46.28882 [error] {<<"method=PUT; path=/users/test; status=500; ">>,{error,{error,{case_clause,[[{<<"update_user">>,<<>>}]]},[{chef_db,update,3,[{file,"/var/cache/omnibus/src/oc_erchef/_build/default/lib/chef_db/src/chef_db.erl"},{line,259}]},{oc_chef_wm_base,update_from_json,4,[{file,"/var/cache/omnibus/src/oc_erchef/_build/default/lib/oc_chef_wm/src/oc_chef_wm_base.erl"},{line,1121}]},{webmachine_resource,resource_call,3,[{file,"/var/cache/omnibus/src/oc_erchef/_build/default/lib/webmachine/src/webmachine_resource.erl"},{line,203}]},{webmachine_resource,do,3,[{file,"/var/cache/omnibus/src/oc_erchef/_build/default/lib/webmachine/src/webmachine_resource.erl"},{line,148}]},{webmachine_decision_core,resource_call,1,[{file,"/var/cache/omnibus/src/oc_erchef/_build/default/lib/webmachine/src/webmachine_decision_core.erl"},{line,47}]},{webmachine_decision_core,accept_helper,1,[{file,"/var/cache/omnibus/src/oc_erchef/_build/default/lib/webmachine/src/webmachine_decision_core.erl"},{line,613}]},{webmachine_decision_core,decision,1,[{file,"/var/cache/omnibus/src/oc_erchef/_build/default/lib/webmachine/src/webmachine_decision_core.erl"},{line,518}]},{webmachine_decision_core,handle_request,2,[{file,"/var/cache/omnibus/src/oc_erchef/_build/default/lib/webmachine/src/webmachine_decision_core.erl"},{line,32}]}]}}}

Add user function returns [[<<"add_user">>,<<>>]] that does not get handled in sqerl

(oc_erchef@api)1> [error] {<<"method=POST; path=/users; status=500; ">>,{error,{error,{case_clause,{ok,[[{<<"add_user">>,<<>>}]]}},[{sqerl,statement,4,[{file,"/host/src/oc_erchef/_build/default/lib/sqerl/src/sqerl.erl"},{line,111}]},{timer,tc,1,[{file,"timer.erl"},{line,166}]},{stats_hero,ctime,3,[{file,"/host/src/oc_erchef/_build/default/lib/stats_hero/src/stats_hero.erl"},{line,131}]},{chef_db,create,3,[{file,"/host/src/oc_erchef/_build/dev/lib/chef_db/src/chef_db.erl"},{line,168}]},{oc_chef_wm_base,create_from_json,5,[{file,"/host/src/oc_erchef/_build/dev/lib/oc_chef_wm/src/oc_chef_wm_base.erl"},{line,1061}]},{webmachine_resource,resource_call,3,[{file,"/host/src/oc_erchef/_build/default/lib/webmachine/src/webmachine_resource.erl"},{line,203}]},{webmachine_resource,do,3,[{file,"/host/src/oc_erchef/_build/default/lib/webmachine/src/webmachine_resource.erl"},{line,148}]},{webmachine_decision_core,resource_call,1,[{file,"/host/src/oc_erchef/_build/default/lib/webmachine/src/webmachine_decision_core.erl"},{line,47}]}]}}}

http://wilson.ci.chef.co/job/chef-server-trigger-ad_hoc/10/downstreambuildview/

marcparadise · 2017-08-28T18:02:19Z

src/oc_erchef/schema/deploy/create_and_update_users.sql

+                                       p_admin users.admin%TYPE,
+                                       p_id character(32)) RETURNS integer AS $update_user$
+    BEGIN
+    IF p_public_key != 'thisisnotakey' THEN


Can we just set to null/rely on null here instead of this value?

The reason we are not using null is - null in v0 api implies that the user wants to delete the key, hence the use of a sentinel value.

marcparadise · 2017-08-28T18:03:43Z

src/oc_erchef/schema/revert/keys_update_trigger@users_email_functional_index.sql

@@ -0,0 +1,14 @@
+-- Revert keys_update_trigger


The revert function should set it back to the previous definitons instead of dropping.

right the current keys_update_trigger.sql does that, this is the revert for the file before making my changes.

marcparadise · 2017-08-28T18:11:11Z

src/oc_erchef/schema/revert/keys_update_trigger.sql

+   END;
+$add_key$ LANGUAGE plpgsql;
+
+CREATE OR REPLACE FUNCTION update_key() RETURNS TRIGGER AS $update_key$


recommend rename to update_or_delete_default_key since we also do direct updates to the keys table via other APIs, and it will delete the key if a new one is not provided.

Disregard, it was misnamed from the start and changing it now will complicate the sqitch bits unnecessarily.

marcparadise · 2017-08-28T20:20:53Z

src/oc_erchef/apps/chef_db/src/chef_sql.erl

@@ -1004,7 +1004,7 @@ create_object(insert_cookbook_artifact_version = QueryName, Args) when is_list(A
        {conflict, _Why} = Conflict -> Conflict
    end;
 create_object(QueryName, Args) when is_atom(QueryName), is_list(Args) ->
-    sqerl:statement(QueryName, Args, count);
+    sqerl:select(QueryName, Args, count);


Why did you need to change this to select? Context for concern: creation of most other object types is done via insert statements, and this would affect them as well.

We changed the insert statements to be a function in plpgsql, and those return in the form : {ok,[[{<<"add_user">>,<<>>}]]}
but select expects return in the form: https://github.com/chef/sqerl/blob/master/src/sqerl.erl#L94-L101
which is why the change from statement to select.

updated as discussed!

marcparadise · 2017-08-28T20:25:05Z

src/oc_erchef/schema/sqitch.plan

@@ -80,3 +80,7 @@ node_policy_name_policy_group 2015-09-04T01:15:03Z Daniel DeLeo <ddeleo@lorentz.

 users_email_functional_index [users] 2017-05-30T12:10:32Z Stephan Renatus <[email protected]> # Add functional index for index-based lower(email) lookups
 @users_email_functional_index 2017-05-30T13:27:15Z Stephan Renatus <[email protected]> # Add functional index for index-based lower(email) lookups
+
+create_and_update_users 2017-08-23T22:53:21Z Prajakta Purohit <[email protected]> # Adding functions to create and update users by inserting a sentinel value in the public_key_columns
+keys_update_trigger [keys_update_trigger@users_email_functional_index] 2017-08-23T23:12:29Z Prajakta Purohit <[email protected]> # The insert and update triggers ignore rows with sentinel value for public_key.


The creation of this change as @users_email_functional_index seems off to me since we want it to go in sequence after the previous one - and if sqitch tracks this as dependent on users_email_functional_index, it may not go in that order. Could you verify that this won't be a thing?

That is a direct change due to sqitch rework keys_update_trigger but will make sure.

marcparadise · 2017-08-28T20:40:28Z

src/oc_erchef/apps/chef_db/src/chef_db.erl

@@ -167,6 +166,7 @@ create(ObjectRec0, #context{server_api_version = ApiVersion, reqid = ReqId}, Act
    Fields = chef_object:fields_for_insert(ObjectRec2),
    case stats_hero:ctime(ReqId, {chef_sql, create_object},
                          fun() -> chef_sql:create_object(QueryName, Fields) end) of
+        {ok,[[{<<"add_user">>,<<>>}]]} -> ok;


This is very specific to the add_user query/function. If we are not able to get a return value consistent with other inserts (number of rows inserted), it's worth a closer look at epgsql+sqerl to understand the returned [[{binary(), binary()}]] from stored proc calls. Would it be sufficient to match similar to the below w/ guards:

PrajaktaPurohit · 2017-09-01T00:16:36Z

src/oc_erchef/schema/deploy/create_and_update_users.sql

+                                    p_external_authentication_uid users.external_authentication_uid%TYPE,
+                                    p_recovery_authentication_enabled users.recovery_authentication_enabled%TYPE,
+                                    p_serialized_object users.serialized_object%TYPE,
+                                    p_admin users.admin%TYPE) RETURNS void AS $$


this should return an integer. use the sqerl:select with first_as_scalar, and the function can return into an integer variable. That will let the return format be {ok, 1}

PrajaktaPurohit · 2017-09-01T00:18:06Z

src/oc_erchef/schema/deploy/keys_update_trigger.sql

-     RETURN NULL; -- result is ignored since this is an AFTER trigger
-   END;
+  BEGIN
+    IF NEW.public_key != 'thisisnotakey' THEN


check if we should add a new change instead of reworking the existing change.

PrajaktaPurohit · 2017-09-01T00:19:09Z

src/oc_erchef/schema/deploy/create_and_update_users.sql

+      ELSE
+       DELETE FROM keys WHERE id = p_id AND key_name = 'default';
+      END IF;
+    END IF;


do we really need this if block? it feels like the trigger still does the right thing.

we still need this because we will not be inserting the key into the users table in all the new requests (after this release) so just leaving the trigger in will update 'thisisnotakey' into the keys table. This function does the right things and the trigger will only be useful for older versions of chef-server

In SQL NULL returns NULL for all comparisons. So the DELETE will never fire above I think, but instead NULL != to 'thisisnotakey' will return NULL, which is falsy, and the inner if will never be reached.

CREATE OR REPLACE FUNCTION test(val text) RETURNS text AS $$ DECLARE result text; BEGIN IF val != 'A' THEN IF val IS NOT NULL THEN result := 'NOT A, NOT NULL'; ELSE result := 'NOT A, BUT NULL'; END IF; ELSE result := 'A'; END IF; RETURN result; END; $$;

opscode_chef=# select test(NULL); test ------ A

We probably should have unit test coverage of some of these functions to insure they work as expected. bifrost does some testing of it's stored procedures; maybe that could be used as a model.

That is an awesome catch! updated the cases to fall through to the delete operation when expected.

I will check the unit tests in bifrost and add something to test this functionality.

markan

It would be helpful to have a comment somewhere explaining the migration implications of the way the old triggers and new code interact, and the careful thinking that had to be done to work this solution out.

markan · 2017-08-31T03:25:51Z

src/oc_erchef/apps/chef_db/priv/pgsql_statements.config

-     hash_type, last_updated_by, created_at, updated_at, external_authentication_uid,
-      recovery_authentication_enabled, serialized_object, admin, pubkey_version) VALUES
-      ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, false, 0)">>}.
+  <<"SELECT add_user($1, $2, $3, $4, 'thisisnotakey', 0, $5, $6, $7, $8, $9, $10, $11, $12, $13, false)">>}.


This is a nitpick for sure, but this_is_not_a_key might be more obvious at a glance.

markan · 2017-09-01T02:55:10Z

src/oc_erchef/schema/deploy/create_and_update_users.sql

+                                    p_serialized_object users.serialized_object%TYPE,
+                                    p_admin users.admin%TYPE) RETURNS void AS $$
+  BEGIN
+    INSERT INTO users


It's probably worth explaining the relationship between this code block and the legacy triggers, and explaining when the triggers can go away.

markan · 2017-09-01T04:28:54Z

src/oc_erchef/schema/deploy/create_and_update_users.sql

+      ELSE
+       DELETE FROM keys WHERE id = p_id AND key_name = 'default';
+      END IF;
+    END IF;


In SQL NULL returns NULL for all comparisons. So the DELETE will never fire above I think, but instead NULL != to 'thisisnotakey' will return NULL, which is falsy, and the inner if will never be reached.

CREATE OR REPLACE FUNCTION test(val text) RETURNS text AS $$ DECLARE result text; BEGIN IF val != 'A' THEN IF val IS NOT NULL THEN result := 'NOT A, NOT NULL'; ELSE result := 'NOT A, BUT NULL'; END IF; ELSE result := 'A'; END IF; RETURN result; END; $$;

opscode_chef=# select test(NULL); test ------ A

markan · 2017-09-01T04:31:30Z

src/oc_erchef/schema/deploy/create_and_update_users.sql

+      ELSE
+       DELETE FROM keys WHERE id = p_id AND key_name = 'default';
+      END IF;
+    END IF;


We probably should have unit test coverage of some of these functions to insure they work as expected. bifrost does some testing of it's stored procedures; maybe that could be used as a model.

markan · 2017-09-01T04:32:52Z

src/oc_erchef/schema/deploy/keys_update_trigger.sql

-     RETURN NULL; -- result is ignored since this is an AFTER trigger
-   END;
+  BEGIN
+    IF NEW.public_key != 'thisisnotakey' THEN


See the NULL comparision comment for create_and_update_users.sql above

markan · 2017-09-01T04:33:08Z

src/oc_erchef/schema/deploy/keys_update_trigger.sql

-   END;
+  BEGIN
+    IF NEW.public_key != 'thisisnotakey' THEN
+      IF NEW.public_key IS NOT NULL THEN


See the NULL comparision comment for create_and_update_users.sql above

markan

I like the way this is heading. @chef/chef-server-maintainers do we have any opinion about whether to use sqitch revise as was done here, or to leave the old files alone and revise the functions in a new file. In the past chef server has tended to do the latter, but that can make it hard to figure out what is eventually installed on the system as you have to integrate a whole series of change files.

stevendanna

Looking good so far!

I'd definitely benefit from some documentation either in the commit messages, a comment, or in the HOW_STUFF_WORKS doc about the details of how this works and what the next steps are. For instance, I still find myself asking "wait, what do we need the triggers for at the moment?" when reading this PR.

I think we should add tests at least the following:

That the user never sees "this_is_not_a_key" in a response body.
What happens when the user deletes their 'default' key.

stevendanna · 2017-09-04T14:11:58Z

src/oc_erchef/apps/chef_db/src/chef_db.erl

@@ -168,6 +167,7 @@ create(ObjectRec0, #context{server_api_version = ApiVersion, reqid = ReqId}, Act
    case stats_hero:ctime(ReqId, {chef_sql, create_object},
                          fun() -> chef_sql:create_object(QueryName, Fields) end) of
        {ok, 1} -> ok;
+        {ok, 0} -> <<"Record not created '", ObjectRec2/binary, "'.">>;


The dialyzer spec on this function needs to be updated. Perhaps it would be good to use {error, <<"Record..._ here as you did below. I think this case would be pretty rare as I imagine if we can't create what we are trying to create, we would get an error back rather than a successful query?

stevendanna · 2017-09-04T14:29:59Z

src/oc_erchef/schema/deploy/create_and_update_users.sql

+      WHERE id = p_id AND key_name = 'default';
+        INSERT INTO keys (id, key_name, public_key, key_version, created_at, expires_at)
+        SELECT p_id, 'default', p_public_key, p_pubkey_version, now(), 'infinity'::timestamp
+        WHERE NOT EXISTS (SELECT 1 FROM keys WHERE id = p_id AND key_name = 'default');


I know this is how we are doing "upsert" in the original code, but I think this construction is racy. My guess is that concurrent user updates are rare enough that we can stay with this though.

Ah, I commented on this down below... if it's available on the Postgres version you're using, an ON CONFLICT clause would be better for this.

Alternatively, using CTEs for this might be preferable, as you could contain the operations within a single statement.

You're also in PL/pgSQL, so you could also use exception handling, but that's more expensive.

Unfortunately, I'm not sure we can drop support for 9.2 yet (we should check with product) which means no ON CONFLICT. I think overall using CTEs would also make some of this a bit more readable; Perhaps we can do that as a more easily reviewable follow-up improvement. (Getting a bit worried that there is a lot going on here as this is already code not many know well.)

I have added the new syntax as a comment in there - a TODO for the future!

christophermaier · 2017-09-12T18:17:10Z

src/oc_erchef/schema/deploy/create_and_update_users.sql

+         p_external_authentication_uid, p_recovery_authentication_enabled,
+         p_serialized_object, p_admin, p_pubkey_version) RETURNING 1)
+      SELECT count(*) FROM createduser INTO inserteduser;
+      RETURN inserteduser;


You don't need this indirection of using CTEs to always insert the literal value 1 into a variable.

Why does this function need to return anything if it's always returning 1? Is the desire to indicate whether something was inserted?

yea - to make sure that the insert did happen.

RE: You don't need this indirection of using CTEs to always insert the literal value 1 into a variable. - will update that but maybe as a second pass, if that is ok?

christophermaier · 2017-09-12T18:22:11Z

src/oc_erchef/schema/revert/create_and_update_users.sql

+BEGIN;
+
+DROP FUNCTION IF EXISTS add_user();
+DROP FUNCTION IF EXISTS update_user();


The deploy script implies that these functions existed in the system before, but this revert will not replace those older implementations.

Yes - this is just safeguarding for the future, we don't expect the functions to actually be present in this iteration

christophermaier · 2017-09-12T18:24:54Z

src/oc_erchef/schema/revert/keys_update_trigger.sql

+         WHERE id = NEW.id AND key_name = 'default';
+       INSERT INTO  keys (id, key_name, public_key, key_version, created_at, expires_at)
+         SELECT NEW.id, 'default', NEW.public_key, NEW.pubkey_version, now(), 'infinity'::timestamp
+         WHERE NOT EXISTS (SELECT 1 FROM keys WHERE id = NEW.id AND key_name = 'default');


Looks like you're trying to do an upsert? Can you use the ON CONFLICT clause instead (see https://www.postgresql.org/docs/current/static/sql-insert.html)... not sure what version of PG y'all are on these days).

I saw this was being done above, as well.

added a comment can be changes once we drop support for postgres 9.2/9.3

Actually maybe I will leave this one as is - since it is the revert of the update trigger.

christophermaier · 2017-09-12T18:26:07Z

src/oc_erchef/schema/t/test_create_and_update_users.sql

@@ -0,0 +1,216 @@
+CREATE OR REPLACE FUNCTION test_zzzzcreate_and_update_users()


Why "zzzz"?

wanted that test to run last - will edit.

christophermaier · 2017-09-12T18:28:08Z

src/oc_erchef/schema/t/test_create_and_update_users.sql

+-- v0 add_user tests --
+RETURN QUERY SELECT is(add_user('11111111111111111111111111111111',
+                                '22222222222222222222222222222222',
+                                'aaaa',


For these dummy values, I'd recommend having them describe what they are as much as you can given any constraints that exist on the data.

So if "aaaa" is a username, use "test_user" instead, etc.

christophermaier · 2017-09-12T18:45:39Z

src/oc_erchef/schema/t/test_create_and_update_users.sql

+RETURN QUERY SELECT results_eq(
+    '"after update email v0"',
+    ARRAY['[email protected]'],
+    'after update email address for v0'


I'd recommend stating these messages in terms of the desired behavior you're going after, instead of the mechanics of what is happening. That makes it easier to figure out what the overall intention of the code is.

christophermaier · 2017-09-12T18:51:53Z

src/oc_erchef/schema/deploy/create_and_update_users.sql

+      WHERE id = p_id AND key_name = 'default';
+        INSERT INTO keys (id, key_name, public_key, key_version, created_at, expires_at)
+        SELECT p_id, 'default', p_public_key, p_pubkey_version, now(), 'infinity'::timestamp
+        WHERE NOT EXISTS (SELECT 1 FROM keys WHERE id = p_id AND key_name = 'default');


Ah, I commented on this down below... if it's available on the Postgres version you're using, an ON CONFLICT clause would be better for this.

Alternatively, using CTEs for this might be preferable, as you could contain the operations within a single statement.

You're also in PL/pgSQL, so you could also use exception handling, but that's more expensive.

christophermaier · 2017-09-12T18:55:48Z

src/oc_erchef/schema/deploy/create_and_update_users.sql

+         last_updated_by, created_at, updated_at, external_authentication_uid,
+         recovery_authentication_enabled, serialized_object, admin,
+         pubkey_version)
+       VALUES (p_id, p_authz_id, p_username, p_email, 'this_is_not_a_key',


If the intention is to have the keys table be the source of truth for keys, why keep key-related information in the users table at all? Is this step 1 of a broader refactoring?

yep - sort of a phase1 since on env like hosted we need to support older versions of chef-server.

marcparadise · 2017-09-14T14:20:00Z

src/oc_erchef/apps/chef_db/src/chef_db.erl

@@ -258,6 +258,7 @@ update(#chef_cookbook_version{org_id =OrgId} = Record, #context{reqid = ReqId} =
 update(ObjectRec, #context{reqid = ReqId}, ActorId) ->
    case stats_hero:ctime(ReqId, {chef_sql, do_update},
                          fun() -> chef_sql:update(ObjectRec, ActorId) end) of
+        0 -> {error, <<"Record not updated '", ObjectRec/binary, "'.">>};


It looks like the changes do_update will return {error, Reason} - can we remove this branch from here?

Same question for create, looks like the {ok. 0} return is handled there.

marcparadise · 2017-09-14T14:21:52Z

src/oc_erchef/apps/chef_db/src/chef_sql.erl

+    case sqerl:select(QueryName, UpdateFields, first_as_scalar, [update_user]) of
+        {ok, 1} -> {ok, 1};
+        {ok, 0} -> {error, <<"Record not updated '", (term_to_binary(QueryName))/binary, "'.">>};
+        {ok, none} -> {ok, not_found};


the caller seems is expecting not_found instead of {ok, not_found}.

marcparadise · 2017-09-14T14:25:49Z

src/oc_erchef/schema/deploy/keys_update_trigger.sql

@@ -1,33 +1,43 @@
 -- Deploy keys_update_trigger
 --
 -- This keeps the keys table consistent when the clients and user tables update their keys
--
+-- We need these triggers to maintain compatibility with older versions ok chef-server.


marcparadise · 2017-09-14T14:29:04Z

src/oc_erchef/schema/deploy/keys_update_trigger@users_email_functional_index.sql

+       INSERT INTO  keys (id, key_name, public_key, key_version, created_at, expires_at)
+         SELECT NEW.id, 'default', NEW.public_key, NEW.pubkey_version, now(), 'infinity'::timestamp
+         WHERE NOT EXISTS (SELECT 1 FROM keys WHERE id = NEW.id AND key_name = 'default');
+     ELSE


This is a port of the existing logic -- but it at always makes me say, "What what?" when I read it. Could you add a comment explaining why a null key causes a deletion? (v0 api behavior compatibility)

Signed-off-by: Marc A. Paradise <[email protected]>

The keys table should be the only source of truth New insert/updates to the users table will insert a sentinel value in the public_key column. The old values are left as is. Triggers will act only on the values in the users.public_key that are not the sentinel value. Signed-off-by: Prajakta Purohit <[email protected]>

PrajaktaPurohit requested a review from a team August 24, 2017 00:35

PrajaktaPurohit force-pushed the praj/SUSTAIN-632 branch 4 times, most recently from 17e09c6 to 8c81ce7 Compare August 25, 2017 23:49

PrajaktaPurohit changed the title ~~[WIP] [SUSTAIN-632] Do not update the users table to contain the public key. The keys table should be the only source of truth~~ [SUSTAIN-632] Do not update the users table to contain the public key. The keys table should be the only source of truth Aug 28, 2017

PrajaktaPurohit force-pushed the praj/SUSTAIN-632 branch 2 times, most recently from 34fb1ec to eeafa2f Compare August 28, 2017 19:18

marcparadise requested changes Aug 28, 2017

View reviewed changes

marcparadise reviewed Aug 28, 2017

View reviewed changes

PrajaktaPurohit force-pushed the praj/SUSTAIN-632 branch 5 times, most recently from 36ed4d0 to c84eda4 Compare August 31, 2017 23:43

PrajaktaPurohit commented Sep 1, 2017

View reviewed changes

markan suggested changes Sep 1, 2017

View reviewed changes

PrajaktaPurohit force-pushed the praj/SUSTAIN-632 branch 3 times, most recently from e1a5066 to 03c5d32 Compare September 1, 2017 20:28

markan reviewed Sep 1, 2017

View reviewed changes

stevendanna reviewed Sep 4, 2017

View reviewed changes

PrajaktaPurohit force-pushed the praj/SUSTAIN-632 branch 6 times, most recently from bb170a8 to a1496c5 Compare September 12, 2017 18:52

christophermaier reviewed Sep 12, 2017

View reviewed changes

PrajaktaPurohit force-pushed the praj/SUSTAIN-632 branch from a1496c5 to 8b64df3 Compare September 12, 2017 19:20

PrajaktaPurohit force-pushed the praj/SUSTAIN-632 branch 4 times, most recently from 39d017d to cf03c32 Compare September 13, 2017 16:12

marcparadise requested changes Sep 14, 2017

View reviewed changes

PrajaktaPurohit force-pushed the praj/SUSTAIN-632 branch from cf03c32 to 8231a44 Compare September 14, 2017 15:47

marcparadise approved these changes Sep 14, 2017

View reviewed changes

markan approved these changes Sep 15, 2017

View reviewed changes

Make eunit verbose to help find failures

9d1df30

Signed-off-by: Marc A. Paradise <[email protected]>

PrajaktaPurohit force-pushed the praj/SUSTAIN-632 branch 3 times, most recently from 291b13c to 49d2dcd Compare September 18, 2017 20:10

PrajaktaPurohit force-pushed the praj/SUSTAIN-632 branch from 49d2dcd to d137d5f Compare September 18, 2017 20:19

PrajaktaPurohit merged commit 97b0f16 into master Sep 18, 2017

PrajaktaPurohit deleted the praj/SUSTAIN-632 branch September 18, 2017 20:39

marcparadise mentioned this pull request May 31, 2022

[INFS-168] Split out dev-docs and add notes on what is next #3318

Merged

		@@ -0,0 +1,216 @@
		CREATE OR REPLACE FUNCTION test_zzzzcreate_and_update_users()

[SUSTAIN-632] Do not update the users table to contain the public key. The keys table should be the only source of truth #1383

[SUSTAIN-632] Do not update the users table to contain the public key. The keys table should be the only source of truth #1383

Conversation

PrajaktaPurohit commented Aug 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markan Sep 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markan Sep 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markan left a comment

Choose a reason for hiding this comment

stevendanna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PrajaktaPurohit Sep 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PrajaktaPurohit commented Aug 24, 2017 •

edited

Loading

markan Sep 1, 2017 •

edited

Loading

markan Sep 1, 2017 •

edited

Loading

PrajaktaPurohit Sep 13, 2017 •

edited

Loading