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

Reject promise immediately if variable is not in first message of field updates #1701

Closed
mofojed opened this issue Dec 20, 2023 · 1 comment · Fixed by #1718
Closed

Reject promise immediately if variable is not in first message of field updates #1701

mofojed opened this issue Dec 20, 2023 · 1 comment · Fixed by #1718
Assignees
@wusteven815
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone
January 2024

Comments

@mofojed
Copy link
Member

mofojed commented Dec 20, 2023

Right now there is a 10s timeout when we're fetching an object:

web-client-ui/packages/jsapi-utils/src/ConnectionUtils.ts

Line 37 in 73e0b65

if (definition != null) {

However, if the object isn't in the first message, we should be able to reject immediately. The underlying call though implies that all known objects should be returned in the first message. From https://github.com/deephaven/deephaven-core/blob/6d203ab9dfbe1fbe203be161d340b8ee0c2e29ed/proto/proto-backplane-grpc/src/main/proto/deephaven/proto/application.proto#L27

The first received message contains all fields that are currently available on the worker. None of these fields will be RemovedFields.
@mofojed mofojed added enhancement New feature or request triage Issue requires triage labels Dec 20, 2023
@vbabich vbabich added this to the January 2024 milestone Jan 2, 2024
@vbabich vbabich added good first issue Good for newcomers and removed triage Issue requires triage labels Jan 2, 2024
@mofojed mofojed self-assigned this Jan 8, 2024
@mofojed
Copy link
Member Author

mofojed commented Jan 8, 2024

Good first issue for @wusteven815

@mofojed mofojed assigned wusteven815 and unassigned mofojed Jan 8, 2024
@wusteven815 wusteven815 mentioned this issue Jan 8, 2024
Merged
@wusteven815 wusteven815 linked a pull request Jan 8, 2024 that will close this issue
feat: Reject promise immediately if var not found #1718
Merged
wusteven815 added a commit that referenced this issue Jan 11, 2024
@wusteven815 @mofojed
feat: Reject promise immediately if var not found (#1718)
43d40bd 
- Implements #1701 
- `fetchVariableDefinition` now throws an `Error` when a variable is not
found (instead of throwing `TimeoutError`)
- `TimeoutError` still used and will be thrown on an actual timeout
- Tests added to check for the correct error

---------

Co-authored-by: Mike Bender <mikebender@deephaven.io>
mofojed pushed a commit to deephaven/deephaven-core that referenced this issue Jan 17, 2024
@github-actions @deephaven-internal
Update web version 0.59.0 (#5044)
b99c015 
# [0.59.0](deephaven/web-client-ui@v0.58.0...v0.59.0) (2024-01-17)


### Bug Fixes

* GoToRow timestamp fails when selected row is out of view ([#1717](deephaven/web-client-ui#1717)) ([9ddc973](deephaven/web-client-ui@9ddc973)), closes [#1561](deephaven/web-client-ui#1561)
* Interface for IrisGridTableModelTemplate.backgroundColorForCell ([#1699](deephaven/web-client-ui#1699)) ([73e1837](deephaven/web-client-ui@73e1837)), closes [#1697](deephaven/web-client-ui#1697)
* Moved logos so they show in production build ([#1713](deephaven/web-client-ui#1713)) ([a3bea73](deephaven/web-client-ui@a3bea73)), closes [#1712](deephaven/web-client-ui#1712)
* re-colorize command codeblocks when theme changes ([#1731](deephaven/web-client-ui#1731)) ([b1e42f5](deephaven/web-client-ui@b1e42f5))
* TimeInput not triggering onChange on incomplete values ([#1711](deephaven/web-client-ui#1711)) ([6894d96](deephaven/web-client-ui@6894d96)), closes [#1710](deephaven/web-client-ui#1710)


### Features

* Action button tooltips ([#1706](deephaven/web-client-ui#1706)) ([bff6bf9](deephaven/web-client-ui@bff6bf9)), closes [#1705](deephaven/web-client-ui#1705)
* Add support for useDeferredApi ([#1725](deephaven/web-client-ui#1725)) ([51ebe1b](deephaven/web-client-ui@51ebe1b))
* Improved preload variable handling ([#1723](deephaven/web-client-ui#1723)) ([ed41c42](deephaven/web-client-ui@ed41c42)), closes [#1695](deephaven/web-client-ui#1695) [#1679](deephaven/web-client-ui#1679)
* NavTabList component ([#1698](deephaven/web-client-ui#1698)) ([96641fb](deephaven/web-client-ui@96641fb))
* Reject promise immediately if var not found ([#1718](deephaven/web-client-ui#1718)) ([43d40bd](deephaven/web-client-ui@43d40bd)), closes [#1701](deephaven/web-client-ui#1701)
* theming tweaks ([#1727](deephaven/web-client-ui#1727)) ([f919a7e](deephaven/web-client-ui@f919a7e))


### BREAKING CHANGES

* - Subclasses of IrisGridTableModelTemplate or it's subclasses that use
backgroundColorForCell may need to update their signature to accept the
theme if they are calling the superclass


Co-authored-by: deephaven-internal <deephaven-internal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wusteven815 wusteven815

Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
January 2024
Development

Successfully merging a pull request may close this issue.

feat: Reject promise immediately if var not found
3 participants
@vbabich @mofojed @wusteven815