Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App ws auth #240

Merged
merged 11 commits into from
Apr 25, 2024
Merged

App ws auth #240

merged 11 commits into from
Apr 25, 2024

Conversation

ThetaSinner
Copy link
Contributor

No description provided.

@ThetaSinner ThetaSinner requested a review from jost-s April 25, 2024 18:16
@@ -1298,28 +1287,3 @@ test(
t.equal(response, "hi", "updated coordinator zomes can be called");
})
);

test(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't find a sensible way to retain this. I think if the websocket connection drops then the app needs to reconnect

@@ -65,7 +65,7 @@ jobs:
run: nix develop -c $SHELL -c "npm ci"

- name: Run tests
run: nix develop -c $SHELL -c "npm t"
run: nix develop --override-input versions/holochain "github:holochain/holochain/holochain-0.3.0-beta-dev.48" -c $SHELL -c "npm t"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until Holonix is updated we need to test against a specific Holochain that is newer than the one in weekly.

As we work up to automatically testing new releases before releasing Holonix updates, we should find a way to make this simpler than changing the workflow file... Not sure how!

Copy link
Contributor

@jost-s jost-s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Groovy!

@@ -32,7 +32,8 @@
],
"scripts": {
"lint": "eslint --fix --ext .ts src test .eslintrc.cjs",
"test:app-agent": "RUST_LOG=error RUST_BACKTRACE=1 tsx test/e2e/app-agent-websocket.ts",
"format": "prettier --write \"src/**/*.ts\" \"test/**/*.ts\"",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

src/api/client.ts Outdated Show resolved Hide resolved
@@ -19,7 +19,7 @@ export type Requester<Req, Res> = (req: Req, timeout?: number) => Promise<Res>;
/**
* @public
*/
export type RequesterUnit<Res> = () => Promise<Res>;
export type RequesterNoArg<Res> = (timeout?: number) => Promise<Res>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a rename from Unit (Rust) to NoArg (JS)? Now it has an arg ;-) Anyhow, not that important

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks diff tool! RequesterUnit wasn't used so I deleted it. This one is supposed to be "no request argument"

DisableCloneCellRequest,
DisableCloneCellResponse
>;
export interface AppAgentClient {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can rename all AppAgent* to App*. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heck, had moved this stuff without looking too closely - hadn't realised how many instances there were of 'agent' still. On it!

/**
* @public
*/
export type AppAgentCallZomeRequest =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename here too?

ThetaSinner and others added 3 commits April 25, 2024 20:08
Co-authored-by: Jost Schulte <jost.schulte@protonmail.com>
@ThetaSinner ThetaSinner merged commit 880c9a3 into develop Apr 25, 2024
10 checks passed
@ThetaSinner ThetaSinner deleted the app-ws-auth branch April 25, 2024 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants