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.

Sign up for GitHub

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

Jump to bottom

feat: Display remote workers in webserver dashboard #848

Merged
merged 5 commits into from
Nov 22, 2023
Merged

feat: Display remote workers in webserver dashboard #848

merged 5 commits into from
Nov 22, 2023

Conversation

liangfung
Copy link
Collaborator

Close #782

@liangfung liangfung force-pushed the feat-display-remote-workers branch from e7c00c7 to 0038d85 Compare November 21, 2023 13:10
@wsxiaoys wsxiaoys added this to the Tabby 0.6.0 milestone Nov 21, 2023
wsxiaoys
wsxiaoys reviewed Nov 21, 2023
View reviewed changes
ee/tabby-ui/lib/tabby-gql-client.ts Outdated
export const graphQLClient = new GraphQLClient(
`${process.env.NEXT_PUBLIC_TABBY_SERVER_URL ?? ''}/graphql`,
{
credentials: 'include',
Copy link
Member

@wsxiaoys wsxiaoys Nov 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

After patching: #858
cors work after removing this line - please research and see what's the reason behind.

@wsxiaoys
Copy link
Member

I encounter following warning when running next dev:

 ⚠ ./node_modules/node-fetch/lib/index.js
Module not found: Can't resolve 'encoding' in '/Users/meng/Projects/tabby/ee/tabby-ui/node_modules/node-fetch/lib'

Import trace for requested module:
./node_modules/node-fetch/lib/index.js
./node_modules/cross-fetch/dist/node-ponyfill.js
./node_modules/graphql-request/build/esm/index.js
./lib/tabby-gql-client.ts
./lib/hooks/use-graphql.ts
./lib/hooks/use-remote-worker.ts
./app/(dashboard)/page.tsx

@wsxiaoys wsxiaoys marked this pull request as draft November 21, 2023 23:00
@liangfung liangfung force-pushed the feat-display-remote-workers branch from d2e9a3e to 0f40cda Compare November 22, 2023 09:13
wsxiaoys
wsxiaoys reviewed Nov 22, 2023
View reviewed changes
ee/tabby-ui/lib/hooks/use-worker.ts Outdated
Copy link
Member

Choose a reason for hiding this comment

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

nit: file name should be use-workers

wsxiaoys
wsxiaoys reviewed Nov 22, 2023
View reviewed changes
ee/tabby-ui/lib/hooks/use-worker.ts Outdated
@@ -32,16 +26,16 @@ function transformHealthInfoToWorker(
}
}

function useMergedWorkers(healthInfo: HealthInfo | undefined) {
const { data } = useRemoteWorkers()
function useWorkers(healthInfo?: HealthInfo) {
Copy link
Member

Choose a reason for hiding this comment

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

just call useHealthInfo here?

@liangfung
Copy link
Collaborator Author

I encounter following warning when running next dev:

 ⚠ ./node_modules/node-fetch/lib/index.js
Module not found: Can't resolve 'encoding' in '/Users/meng/Projects/tabby/ee/tabby-ui/node_modules/node-fetch/lib'

Import trace for requested module:
./node_modules/node-fetch/lib/index.js
./node_modules/cross-fetch/dist/node-ponyfill.js
./node_modules/graphql-request/build/esm/index.js
./lib/tabby-gql-client.ts
./lib/hooks/use-graphql.ts
./lib/hooks/use-remote-worker.ts
./app/(dashboard)/page.tsx

graffle-js/graffle#448
graffle-js/graffle#597
@wsxiaoys The latest version of graphql-request still has the cross-fetch polyfill. The simple way to remove the harmless warning is installing a devDependency encoding. Or just remove graphql-request

@wsxiaoys
Copy link
Member

I encounter following warning when running next dev:

 ⚠ ./node_modules/node-fetch/lib/index.js
Module not found: Can't resolve 'encoding' in '/Users/meng/Projects/tabby/ee/tabby-ui/node_modules/node-fetch/lib'

Import trace for requested module:
./node_modules/node-fetch/lib/index.js
./node_modules/cross-fetch/dist/node-ponyfill.js
./node_modules/graphql-request/build/esm/index.js
./lib/tabby-gql-client.ts
./lib/hooks/use-graphql.ts
./lib/hooks/use-remote-worker.ts
./app/(dashboard)/page.tsx

jasonkuhrt/graphql-request#448 jasonkuhrt/graphql-request#597 @wsxiaoys The latest version of graphql-request still has the cross-fetch polyfill. The simple way to remove the harmless warning is installing a devDependency encoding. Or just remove graphql-request

I’m fine either way - as long as the warning can be silented.

@wsxiaoys wsxiaoys marked this pull request as ready for review November 22, 2023 10:52
@wsxiaoys wsxiaoys merged commit f1e82d6 into TabbyML:main Nov 22, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wsxiaoys wsxiaoys wsxiaoys left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Tabby 0.6.0
Development

Successfully merging this pull request may close these issues.

Display remote workers in webserver dashboard
2 participants
@liangfung @wsxiaoys