Skip to content

Conversation

@jameskerr
Copy link
Contributor

@jameskerr jameskerr commented Jun 18, 2024

Fixes #3092
Fixes #3100

  • Disable Pagination (i.e., limited to 500 rows of query output in window)
  • Disable Drill Down (i.e., right-click Pivot to Values)
  • Disable Enhanced Error Reporting (e.g., red squiggly underlines for syntax errors in the editor)
  • Disable Correlations views in Details side panel & window

The app in this PR does a feature detection step whenever the lake changes or when it boots up for the first time. It will check if the describe endpoint returns a 404. If so, it will disable the following checkboxes above.

Everything seems to be working as intended in this.

useLayoutEffect(() => {
if (status) return
if (lake) dispatch(updateStatus(lake.id))
Active.lake.sync()
Copy link
Contributor Author

@jameskerr jameskerr Jun 18, 2024

Choose a reason for hiding this comment

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

Whenever the current lake id changes, or it's connection status changes, sync what info we have with the lake server.

switch (status) {
case "disconnected":
return <ConnectionError onRetry={() => dispatch(initCurrentTab())} />
return <ConnectionError onRetry={() => dispatch(updateStatus(lake.id))} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

initCurrentTab only called this updateStatus function. So I inlined it and removed initCurrentTab.

All of this "lake" status/authentication code needs another look at some point.

const id = lakeId || defaultLake().id
// This sets up the lake's tabs
const lake = Lake.find(id)
lake.sync()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As soon as the app boots up, we sync the lake.

api.init(store.dispatch, store.getState)
initDOM()
initIpcListeners(store)
initDomainModels({store})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the Domain Models need to be provisioned with the store variable before we call "initLake". So I re-ordered the steps here.

authData?: AuthData
features?: {
describe: boolean
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to add a "features" property the the data we cache about the lake. The only feature right now will be "describe".

static get lake() {
const id = this.select(Current.getLakeId)
return Lake.find(id)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a shortcut to get the Active lake object. Active.lake


type LakeModelAttrs = LakeAttrs & {status: LakeStatus}

export class Lake extends DomainModel<LakeModelAttrs> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we are adding the methods we need to the Lake model.

I've added:

  • find(id)
  • client
  • features
  • sync()
  • update
  • save
  • isRunning()

@@ -0,0 +1,21 @@
import {Lake} from "../lake"

export class FeatureDetector {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This class is responsible for detecting all the features we want.

const program = this.select(Current.getQueryText)
const history = this.select(Current.getHistory)

if (!Active.lake.features.describe) {
Copy link
Contributor Author

@jameskerr jameskerr Jun 18, 2024

Choose a reason for hiding this comment

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

And finally, if the lake does not have describe, do the minimum required and return early.

dontRejectError: true,
});
return result.json();
if (response.ok || response.status === 400) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re-arranged the code to allow for an error response from the server. If the describe endpoint returns code 400, there is a problem with the query, but we don't want to throw an error. We want to report the problem to the user. So we parse the JSON as an expected response.

duplex?: 'half';
dontRejectError?: boolean;
}) {
protected request(opts: Types.RequestOpts) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I separated the creation of the request into this request method. The existing send method then calls this.

The describeQuery method only uses request and handles errors in a different way than the other methods.

@jameskerr jameskerr requested review from mattnibs and philrz and removed request for philrz June 18, 2024 23:35
@philrz

This comment was marked as resolved.

@philrz
Copy link
Contributor

philrz commented Jun 20, 2024

I'm currently testing this branch at commit 62a6084.

The attached video shows a successful walk-through of the functionality that these changes disable when accessing an older Zed lake service. As prep, I have the ZNG format Zed sample data loaded to both the lake running behind Zui (Zed commit 13d15d3) and a separate "remote" Zed v1.14.0 lake I've started on port 8888. As a baseline I first demo each of the four functional areas described by the checkboxes in the opening PR text while accessing the data in the local lake, then I access the v.14.0 lake and confirm that each of these is disabled.

Demo.mp4

@jameskerr jameskerr merged commit 10d8974 into main Jun 20, 2024
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.

Crash when accessing remote lake (broke at 2473666) "Error: 404 page not found" when querying an older Zed lake service

3 participants