Skip to content

Conversation

Raubzeug
Copy link
Contributor

@Raubzeug Raubzeug commented Feb 24, 2025

closes #1970
stand

CI Results

Test Status: βœ… PASSED

πŸ“Š Full Report

Total Passed Failed Flaky Skipped
262 262 0 0 0

😟 No changes in tests. πŸ˜•

Bundle Size: πŸ”Ί

Current: 80.62 MB | Main: 80.61 MB
Diff: +0.01 MB (0.01%)

⚠️ Bundle size increased. Please review.

ℹ️ CI Information
  • Test recordings for failed tests are available in the full report.
  • Bundle size is measured for the entire 'dist' directory.
  • πŸ“Š indicates links to detailed reports.
  • πŸ”Ί indicates increase, πŸ”½ decrease, and βœ… no change in bundle size.

@Raubzeug Raubzeug force-pushed the preserve-database-param branch from f5adb59 to ac89a5a Compare February 25, 2025 14:35
Comment on lines +70 to +75
const locationSearch = new URLSearchParams(window.location.search);
const database = locationSearch.get('database');
if (database) {
extendedQuery = {...extendedQuery, database};
}

Copy link
Member

Choose a reason for hiding this comment

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

You should only add database if there is no database in initial query. So it should be like

extendedQuery = {database, ...extendedQuery}

wrapTo={({id}, node) => {
const path = getTenantPath({
...queryParams,
database: tenantName,
Copy link
Member

Choose a reason for hiding this comment

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

You already have database in queryParams, ...queryParams is enough

tabletInfo.push({
label: tabletInfoKeyset('field_hive'),
value: (
//TODO: add database to getTabletPagePath after fix in backend
Copy link
Member

Choose a reason for hiding this comment

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

What fix on backend is needed?

Comment on lines +131 to +133
tenantName,
location,
tenantName,
Copy link
Member

Choose a reason for hiding this comment

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

tenantName is passed twice

Comment on lines +190 to +191
// TODO: add database when backend support it
// database={database?.toString()}
Copy link
Member

Choose a reason for hiding this comment

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

What support from backend is needed?

{
node_id: nodeId,
pdisk_id: pDiskId,
database,
Copy link
Member

Choose a reason for hiding this comment

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

PDisk doesn't relate to specific database

@artemmufazalov artemmufazalov marked this pull request as draft March 3, 2025 12:32
@artemmufazalov artemmufazalov added the status/need info Further information is requested label Mar 3, 2025
@artemmufazalov artemmufazalov marked this pull request as ready for review June 9, 2025 16:21
@artemmufazalov artemmufazalov deleted the preserve-database-param branch June 9, 2025 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/need info Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Always preserve "database" query parameter inside database

2 participants