Skip to content

ui: fix Overview screen in OSS builds#56591

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
oxidecomputer:ui-oss-not-found
Nov 13, 2020
Merged

ui: fix Overview screen in OSS builds#56591
craig[bot] merged 1 commit intocockroachdb:masterfrom
oxidecomputer:ui-oss-not-found

Conversation

@davepacheco
Copy link
Copy Markdown

Previously, when using OSS builds (created with make buildoss), when
you loading the DB Console in your browser, you'd get "Page Not Found".
The route for the overview page was missing the leading '/'. This bug
appears to have been introduced in
722c932.

Release note (admin ui change): This fixes a bug where users of
the OSS builds of CockroachDB would see "Page Not Found" when loading
the Console.

@davepacheco davepacheco requested a review from a team November 11, 2020 23:32
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Nov 11, 2020

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@cockroach-teamcity
Copy link
Copy Markdown
Member

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@blathers-crl blathers-crl bot added the O-community Originated from the community label Nov 11, 2020
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@davepacheco
Copy link
Copy Markdown
Author

Debugging and fix by @jclulow. All I did was find the bug and package up the fix that we found to work. This passes make ui-lint and make buildoss produces a build where the Console screen works instead of saying "Page Not Found".

@davepacheco
Copy link
Copy Markdown
Author

I have signed the CLA several minutes ago but it still says pending, even after I click "recheck".

@dhartunian
Copy link
Copy Markdown
Collaborator

Hi @davepacheco and @jclulow! Thank you for your contribution. Let me look into the CLA issue and see what's going on.

@dhartunian dhartunian self-requested a review November 12, 2020 15:32
@dhartunian
Copy link
Copy Markdown
Collaborator

@davepacheco can you try the fix described here #55202 (review) and see if that helps? Apologies for the trouble.

Copy link
Copy Markdown
Collaborator

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @dhartunian)

@dhartunian
Copy link
Copy Markdown
Collaborator

I'll merge this via bors once CLA is green.

Previously, when using OSS builds (created with `make buildoss`), when
you loading the DB Console in your browser, you'd get "Page Not Found".
The route for the overview page was missing the leading '/'.  This bug
appears to have been introduced in
722c932.

Release note (admin ui change): This fixes a bug where users of
the OSS builds of CockroachDB would see "Page Not Found" when loading
the Console.
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Nov 12, 2020

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@davepacheco
Copy link
Copy Markdown
Author

Thanks @dhartunian. I tried both fixes in the comment that you linked to. The first failed like this:

$ curl -H "Authorization: token $token" -d '{"state": "success", "context":"license/cla", "description": "curl"}' https://api.github.com/repos/cockroachdb/cockroach/statuses/cc1c94eb5119dad49935ba2e0121c2a17fc8eab7
{
  "message": "Not Found",
  "documentation_url": "https://docs.github.com/rest/reference/repos#create-a-commit-status"
}

That documentation URL implied you can only do this with push access to the repo, so I gave up on that.

The other suggestion was squash and force push. I just did that, though it doesn't look like the CLA status has been updated. Let me know if there's something else for me to try.

@dhartunian
Copy link
Copy Markdown
Collaborator

appreciate your patience @davepacheco. I confirmed that the CLA is signed and we did the override for the github check.

@dhartunian
Copy link
Copy Markdown
Collaborator

bors r+

craig bot pushed a commit that referenced this pull request Nov 12, 2020
55325: ui: Transactions link between Sessions and Statements r=dhartunian a=elkmaster

Resolves: #55131, #56244

Release note (admin ui change): Link to the Transactions page is now shown
between the Sessions and Statements links in the left hand navigation. This more
clearly reflects the hierarchy between the 3 concepts.

56346: testcluster: minor logging improvements r=andreimatei a=andreimatei

Log when TestCluster quiescing starts, and add a node log tag to each
node's quiescing ctx so messages from different nodes can be
disambiguated.

Release note: None

56437: cli, ui: dismiss release notes signup banner per environment variable r=knz,dhartunian a=nkodali

Previously, the signup banner could only be dismissed manually.
For internal testing purposes, this banner is unnecessary. This
change provides a way to dismiss the signup banner upon start of
a cluster via the cli by setting the environment variable
COCKROACH_UI_RELEASE_NOTES_SIGNUP_DISMISSED=true.

Resolves #46998

Release note: none

56533: backupccl: add feature flag support for BACKUP, RESTORE r=otan a=angelapwen

Follow-up to the RFC at #55778. This addresses the SRE use case mentioned in #51643 — instead of moving forward with a global denylist as the RFC indicated, we are prototyping feature flags via cluster settings to turn on/off requested features. The first part of the prototype will be done on `BACKUP` and `RESTORE` commands.

See [this doc](https://docs.google.com/document/d/1nZSdcK7YprL0P4TAuseY-mvlYnd82IaJ_ptAQDoWB6o/edit?) for further details. 

Note that the logic test under `ccl/backupccl/testdata/backup-restore/feature-flags` can be tested with the command `make test PKG=./pkg/ccl/backupccl TESTS='TestBackupRestoreDataDriven'`

— Commit message below — 

Adds a cluster setting to toggle a feature flag for the BACKUP and
RESTORE commands off and on; as well as a broad category for
Bulk IO commands. Currently disabling the cluster setting for Bulk
IO will only disable BACKUP and RESTORE jobs, but other types may
be included in this category in the future..

The feature is being introduced to address a Cockroach Cloud SRE
use case: needing  to disable certain categories of features in
case of cluster failure.

Release note (enterprise change): Adds cluster settings to enable/
disable the BACKUP and RESTORE commands. If a user attempts to use
these features while they are disabled, an error indicating that
the database administrator has disabled the feature is surfaced.

Example usage for the database administrator:
SET CLUSTER SETTING feature.bulkio.backup.enabled = FALSE;
SET CLUSTER SETTING feature.bulkio.backup.enabled = TRUE;
SET CLUSTER SETTING feature.bulkio.restore.enabled = FALSE;
SET CLUSTER SETTING feature.bulkio.restore.enabled = TRUE;
SET CLUSTER SETTING feature.bulkio.enabled = FALSE;
SET CLUSTER SETTING feature.bulkio.enabled = TRUE;

56591: ui: fix Overview screen in OSS builds r=dhartunian a=davepacheco

Previously, when using OSS builds (created with `make buildoss`), when
you loading the DB Console in your browser, you'd get "Page Not Found".
The route for the overview page was missing the leading '/'.  This bug
appears to have been introduced in
722c932.

Release note (admin ui change): This fixes a bug where users of
the OSS builds of CockroachDB would see "Page Not Found" when loading
the Console.

56600: roachpb: remove SetInner in favor of MustSetInner r=nvanbenschoten a=tbg

As of a recent commit, `ErrorDetail.SetInner` became unused, and
we can switch to a `MustSetInner` pattern for `ErrorDetail`. Since
the codegen involved is shared with {Request,Response}Union, those
lose the `SetInner` setter as well; we were always asserting on
the returned bool there anyway so this isn't changing anything.

Release note: None



Co-authored-by: Vlad Los <carrott9@gmail.com>
Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
Co-authored-by: Namrata Kodali <namrata@cockroachlabs.com>
Co-authored-by: angelapwen <angelaw@cockroachlabs.com>
Co-authored-by: Joshua M. Clulow <josh@sysmgr.org>
Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 12, 2020

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 13, 2020

Build succeeded:

@craig craig bot merged commit e640435 into cockroachdb:master Nov 13, 2020
@davepacheco
Copy link
Copy Markdown
Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

O-community Originated from the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants