Update InstallAppRequest type to new format #298

matthme · 2024-11-25T12:37:39Z

This PR contains the type changes required in order to adhere to holochain/holochain#4416 and its follow-up PR holochain/holochain#4488

jost-s

Please add a test or modify an existing one that uses roles_settings.

matthme · 2024-11-25T22:05:14Z

I'm confused - why do the tests even pass? Are the tests not actually ran in CI?

jost-s · 2024-11-25T22:10:25Z

The tests pass because memproofs is just ignored and roles_settings is optional.

…tings set

matthme · 2024-11-26T15:14:41Z

Test added.

matthme · 2024-11-26T15:31:19Z

Hmm...actually the problem that Uint8Arrays don't deserialize to YamlProperties is a problem. This means that you can't even pass in agent public keys there unless you base64 encode them or conver them to an Array<number>.

matthme · 2024-11-26T18:07:23Z

test/e2e/index.ts

+          membrane_proof: new Uint8Array(6),
+          modifiers: {
+            network_seed: "hello",
+            properties: yaml.dump({ progenitor: progenitorKey }),


Changed the test now to use a yaml string.
cc @ThetaSinner

matthme · 2024-11-26T18:09:00Z

Okay never mind. I think this is totally appropriate and the app developer can be expected to take care of any required conversion for example using js-yaml (we just need to make sure that the js-client actually returns a useful error which is not currently the case #299).

jost-s

LGTM
a suggestion for the yaml properties type and some cleanup

jost-s · 2024-11-26T22:02:40Z

package.json

@@ -48,13 +48,15 @@
    "isomorphic-ws": "^5.0.0",
    "js-base64": "^3.7.5",
    "js-sha512": "^0.9.0",
+    "js-yaml": "4.1.0",


This can be a dev dependency too.

jost-s · 2024-11-26T22:04:14Z

src/api/admin/types.ts

+ *  @public
+ * Any value that is serializable to a Yaml value
+ */
+// eslint-disable-next-line @typescript-eslint/no-explicit-any


Is this needed here? It should only produce a warning.
Actually unknown would be the idiomatic type.

jost-s · 2024-11-26T22:05:52Z

test/e2e/common.ts

@@ -117,7 +117,7 @@ export const installAppAndDna = async (
    installed_app_id,
    agent_key: agent,
    path,
-    membrane_proofs: {},
+    roles_settings: {},


roles_settings is optional

Suggested change

roles_settings: {},

jost-s · 2024-11-26T22:06:03Z

test/e2e/common.ts

@@ -159,7 +159,7 @@ export const createAppWsAndInstallApp = async (
    installed_app_id,
    agent_key: agent,
    path,
-    membrane_proofs: {},
+    roles_settings: {},


Suggested change

roles_settings: {},

I'm still here =)

And still here

jost-s · 2024-11-26T22:06:30Z

test/e2e/index.ts

@@ -85,7 +87,7 @@ test(
      installed_app_id,
      agent_key,
      path: `${FIXTURE_PATH}/test.happ`,
-      membrane_proofs: {},
+      roles_settings: {},


and so on further down

Suggested change

roles_settings: {},

jost-s · 2024-11-26T22:07:29Z

test/e2e/index.ts

@@ -1046,7 +1119,7 @@ test(

    const tag = "test_tag";
    const link: Link = await client.callZome({
-      cap_secret: null,
+      cap_secret: undefined,


Suggested change

cap_secret: undefined,

…pubkey to Uint8Array to have deepEqual work properly

matthme · 2024-11-26T22:27:08Z

Thank you! Addressed all of these now.

jost-s

Just a few more bits

jost-s · 2024-11-26T22:42:37Z

Cargo.toml

@@ -7,4 +7,4 @@ resolver = "2"
 opt-level = "z"

 [workspace.dependencies]
-hdk = "0.5.0-dev.0"
+hdk = "0.5.0-dev.6"


This is intentionally kept at .0 as long as there are no breaking changes in the HDK.

jost-s · 2024-11-26T22:44:10Z

test/e2e/common.ts

@@ -159,7 +159,7 @@ export const createAppWsAndInstallApp = async (
    installed_app_id,
    agent_key: agent,
    path,
-    membrane_proofs: {},
+    roles_settings: {},


I'm still here =)

matthme · 2024-11-28T17:06:32Z

search and replace for the win.

jost-s · 2024-11-28T18:08:09Z

test/e2e/common.ts

@@ -159,7 +159,7 @@ export const createAppWsAndInstallApp = async (
    installed_app_id,
    agent_key: agent,
    path,
-    membrane_proofs: {},
+    roles_settings: {},


And still here

jost-s · 2024-11-28T18:09:24Z

test/e2e/common.ts

@@ -117,7 +117,7 @@ export const installAppAndDna = async (
    installed_app_id,
    agent_key: agent,
    path,
-    membrane_proofs: {},
+    roles_settings: {},


* update InstallAppRequest type to new format * add new YamlProperties type and test that installs app with roles_settings set * use js-yaml in test * remove unnecessary fields, remove js-yaml from dependencies, convert pubkey to Uint8Array to have deepEqual work properly * more redundant membrane_proof fields, revert hdk bump * removed unused roles_settings fields

* Update InstallAppRequest type to new format (#298) * update InstallAppRequest type to new format * add new YamlProperties type and test that installs app with roles_settings set * use js-yaml in test * remove unnecessary fields, remove js-yaml from dependencies, convert pubkey to Uint8Array to have deepEqual work properly * more redundant membrane_proof fields, revert hdk bump * removed unused roles_settings fields * rebuild docs * removed .only to have all tests run * bump hdk verison, fix hc sandbox args and log errors, update flake * test: fix expected number of cells due to dpki being removed * revert hdk bump --------- Co-authored-by: Jost Schulte <jost-s@users.noreply.github.com>

update InstallAppRequest type to new format

33ead65

matthme added the backport-0.4 label Nov 25, 2024

matthme requested a review from a team November 25, 2024 15:13

jost-s requested changes Nov 25, 2024

View reviewed changes

add new YamlProperties type and test that installs app with roles_set…

9539e9e

…tings set

matthme mentioned this pull request Nov 26, 2024

Make it possible to install an app with custom properties holochain/holochain#3918

Closed

6 tasks

matthme requested a review from jost-s November 26, 2024 15:14

use js-yaml in test

f4e52c7

matthme commented Nov 26, 2024

View reviewed changes

jost-s requested changes Nov 26, 2024

View reviewed changes

remove unnecessary fields, remove js-yaml from dependencies, convert …

50043a9

…pubkey to Uint8Array to have deepEqual work properly

matthme requested a review from jost-s November 26, 2024 22:40

jost-s requested changes Nov 26, 2024

View reviewed changes

more redundant membrane_proof fields, revert hdk bump

e868ecb

matthme requested a review from jost-s November 28, 2024 17:06

jost-s requested changes Nov 28, 2024

View reviewed changes

removed unused roles_settings fields

b5ad439

jost-s approved these changes Nov 28, 2024

View reviewed changes

matthme merged commit 43d5e9e into main Nov 28, 2024
10 checks passed

matthme deleted the new-install-app-payload branch November 28, 2024 19:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update InstallAppRequest type to new format #298

Update InstallAppRequest type to new format #298

matthme commented Nov 25, 2024

jost-s left a comment

matthme commented Nov 25, 2024

jost-s commented Nov 25, 2024

matthme commented Nov 26, 2024

matthme commented Nov 26, 2024

matthme Nov 26, 2024

matthme commented Nov 26, 2024

jost-s left a comment

jost-s Nov 26, 2024

jost-s Nov 26, 2024

jost-s Nov 26, 2024 •

edited

Loading

jost-s Nov 28, 2024

matthme Nov 28, 2024

jost-s Nov 26, 2024

jost-s Nov 26, 2024

jost-s Nov 28, 2024

jost-s Nov 26, 2024

jost-s Nov 26, 2024

matthme commented Nov 26, 2024

jost-s left a comment

jost-s Nov 26, 2024

matthme Nov 28, 2024

jost-s Nov 26, 2024

matthme commented Nov 28, 2024

jost-s Nov 28, 2024

jost-s Nov 28, 2024

Update InstallAppRequest type to new format #298

Update InstallAppRequest type to new format #298

Conversation

matthme commented Nov 25, 2024

jost-s left a comment

Choose a reason for hiding this comment

matthme commented Nov 25, 2024

jost-s commented Nov 25, 2024

matthme commented Nov 26, 2024

matthme commented Nov 26, 2024

Choose a reason for hiding this comment

matthme commented Nov 26, 2024

jost-s left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jost-s Nov 26, 2024 • 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

matthme commented Nov 26, 2024

jost-s 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

matthme commented Nov 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jost-s Nov 26, 2024 •

edited

Loading