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.

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

Status Dashboard #2090

Merged
merged 65 commits into from
Mar 11, 2024
Merged

Status Dashboard #2090

merged 65 commits into from
Mar 11, 2024

Conversation

afshin
Copy link
Member

@afshin afshin commented Feb 26, 2024

This is an updated status/dashboard page for the conda-forge website.

This PR covers the functionality every core component of the current conda-forge status site.

Please TRY IT OUT, feedback is welcome 🙏

cf. #1971

Copy link

netlify bot commented Feb 26, 2024

Deploy Preview for conda-forge-previews ready!

Name Link
🔨 Latest commit 91469b2
🔍 Latest deploy log https://app.netlify.com/sites/conda-forge-previews/deploys/65ef0b6f1eb9c90008674a08
😎 Deploy Preview https://deploy-preview-2090--conda-forge-previews.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@afshin afshin marked this pull request as ready for review February 26, 2024 15:25
@afshin afshin requested a review from a team as a code owner February 26, 2024 15:25
@afshin
Copy link
Member Author

afshin commented Feb 26, 2024

The CI failure seems to come from trying to access a dynamic URL, but I am not certain. You can see in the deployment that the actual site logic is functional, but there might be something I am missing.

@traversaro
Copy link
Contributor

Please TRY IT OUT, feedback is welcome 🙏

Thanks a lot! As a "normal" contributor, I was wondering if in this new version there is a way to see the error messages for "non-solvable" packages in migration, see for example https://conda-forge.org/status/#libabseil20240116_libgrpc161_libprotobuf4252 .

Copy link
Member

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

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

Added some comments with, mostly, tiny things. The biggest item is that there's no documentation in the code at all. While it's well written and it can be easily navigated in the IDE, I think it would be beneficial to have a big picture description on how the dashboard is built and under what principles. If possible, each module should have a top-level docstring (or whatever the equivalent is in JS) with the purpose and scope of the module.

docusaurus.config.js Show resolved Hide resolved
docusaurus.config.js Outdated Show resolved Hide resolved
docusaurus.config.js Outdated Show resolved Hide resolved
docusaurus.config.js Outdated Show resolved Hide resolved
package.json Outdated
Comment on lines 21 to 33
"@mdx-js/react": "^3.0.0",
"chart.js": "^4.4.0",
"chartjs-adapter-moment": "^1.0.1",
"clsx": "^1.2.1",
"install": "^0.13.0",
"moment": "^2.29.4",
"octokit": "^3.1.2",
"prism-react-renderer": "^2.3.1",
"react": "^18.2.0",
"react-dom": "^18.2.0"
"react-chartjs-2": "^5.2.0",
"react-dom": "^18.2.0",
"react-markdown": "^9.0.1",
"remark-gfm": "^4.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it would be useful to document why these dependencies have been added. For example, is remark-gfm directly used now? Does that mean we can use Github flavoured markdown in the documentation now? Or is it just because it made it easier to write certain parts of the dashboard?

Copy link
Member

Choose a reason for hiding this comment

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

Coming back to this, it looks like what folks do is something along the lines of:

"packages": {
...
},
"// packages": {
  "//": "There's nothing special about this dict; it's just a collection of comments",
  "install": "This package is used for ...",
  ...
}

src/components/StatusDashboard/index.jsx Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

I don't know how common this is in the JS world, but I'm missing the equivalent of module docstrings in almost all files. Having a little bit of technical documentation to see how to navigate the dashboard code would be super helpful.

src/components/MigrationDetails/styles.module.css Outdated Show resolved Hide resolved
src/components/StatusDashboard/current_migrations.jsx Outdated Show resolved Hide resolved
@jaimergp
Copy link
Member

Please TRY IT OUT, feedback is welcome 🙏

Thanks a lot! As a "normal" contributor, I was wondering if in this new version there is a way to see the error messages for "non-solvable" packages in migration, see for example https://conda-forge.org/status/#libabseil20240116_libgrpc161_libprotobuf4252 .

Adding screenshot for clarity:

image

@jaimergp
Copy link
Member

I can't find that functionality either. The URL for that migration would be https://deploy-preview-2090--conda-forge-previews.netlify.app/status/migration/libabseil20240116_libgrpc161_libprotobuf4252. If I filter for unsolvable only, I just get:

image

Weirdly, the package names in the first column are not clickable (they take you nowhere), despite having a hover state. Don't know if bug or intended, but the hover is confusing.

@jaimergp jaimergp mentioned this pull request Feb 26, 2024
18 tasks
@afshin
Copy link
Member Author

afshin commented Feb 26, 2024

Please TRY IT OUT, feedback is welcome 🙏

Thanks a lot! As a "normal" contributor, I was wondering if in this new version there is a way to see the error messages for "non-solvable" packages in migration, see for example https://conda-forge.org/status/#libabseil20240116_libgrpc161_libprotobuf4252 .

You're right, this is a piece of functionality that is missing in the new version. I'll give it some thought and try to incorporate it. (cc: @GabrielaVives)

@afshin
Copy link
Member Author

afshin commented Feb 26, 2024

I can't find that functionality either. The URL for that migration would be https://deploy-preview-2090--conda-forge-previews.netlify.app/status/migration/libabseil20240116_libgrpc161_libprotobuf4252. If I filter for unsolvable only, I just get:

image

Weirdly, the package names in the first column are not clickable (they take you nowhere), despite having a hover state. Don't know if bug or intended, but the hover is confusing.

I see what's happened. Almost all the left-column items have a PR link. Some do not, but they're being automatically linked to empty string. I will put in a check to only link the ones that actually have a URL for the PR.

Copy link
Member

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

I cannot find the solver errors we post to the status page.

Copy link
Member

@beckermr beckermr left a comment

Choose a reason for hiding this comment

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

We also need to integrate a large banner at the top with the status repo issues tracker.

@mfansler
Copy link
Member

image

Really neat! I have some feedback on the migrations views:

  • I found the interaction for filtering by status counterintuitive. I expected clicking "In PR" to filter to only those with that status, rather than filtering out those (since everything is shown by default).

  • The order of the filter buttons and the sets in the bar graph neglect the semantics. The order should be:

    Done > In PR > Awaiting PR > Awaiting parents > Not solvable > Bot error
    

    Note that this order is reflected in the viridis-like rainbow colors.

  • Something that doesn't work on the current dashboard and keeps the same behavior in the update is the hyperlinking of nodes in the graph view. If one opens the graph image in a new tab/window, the hyperlinks become active and let one jump to the PR. This is perhaps the most valuable functionality in this status board for me in terms of moving migrations forward. It would be nice if the hyperlinks worked in the page.

@SylvainCorlay
Copy link
Member

Something that doesn't work on the current dashboard and keeps the same behavior in the update is the hyperlinking of nodes in the graph view. If one opens the graph image in a new tab/window, the hyperlinks become active and let one jump to the PR. This is perhaps the most valuable functionality in this status board for me in terms of moving migrations forward. It would be nice if the hyperlinks worked in the page.

Wow I did not know about this feature!

@jaimergp
Copy link
Member

The CI failure seems to come from trying to access a dynamic URL, but I am not certain. You can see in the deployment that the actual site logic is functional, but there might be something I am missing.

The error is:

[404] https://conda-forge.org/status/migration/ | Failed: Network error: Not Found

Which is expected because this PR hasn't been merged yet so it's actually checking the old single page /status. So we can ignore for now, or remove this line:

--remap "https://conda-forge.org/status https://conda-forge.org/status"

Comment on lines 136 to 166
.status_dashboard_toc {
border: var(--ifm-table-border-width) solid #ccc;
border-radius: 5px;
font-size: 16px;
position: sticky;
top: 80px;
max-width: 250px;
}

.status_dashboard_toc a {
display: inline-block;
min-width: 100%;
text-decoration: none;
}

.status_dashboard_toc ul {
list-style-type: none;
margin: 0;
padding: 0;
}

.status_dashboard_toc li {
background-color: var(--ifm-table-stripe-background);
border-radius: 5px;
padding: 1px 5px;
user-select: none;
}

.status_dashboard_toc li:hover {
background-color: transparent;
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we rework this styling so it matches the sidebar in /docs? It's almost there but there are subtle differences. Otherwise, we could also try the simple link list of /blog.

src/constants.js Outdated Show resolved Hide resolved
src/constants.js Outdated Show resolved Hide resolved
@jaimergp
Copy link
Member

I've also noticed that the migration tables can only be sorted together; e.g. if I sort "long-running migrations" by name, "Regular migrations" and "Closed migrations" also get sorted in the same way:

Screen.Recording.2024-02-27.at.10.20.34.mov

@afshin
Copy link
Member Author

afshin commented Feb 27, 2024 via email

@jaimergp
Copy link
Member

Not necessarily. I was expecting individual behavior given the sort buttons in each subtable, but if it makes everything more complicated, for me it's not personally worth "fixing" it.

@GabrielaVives
Copy link

Not necessarily. I was expecting individual behavior given the sort buttons in each subtable, but if it makes everything more complicated, for me it's not personally worth "fixing" it.

I think that what is counter-intuitive here, is the repetitive column headers that give the impression that there are 3 independent tables.
Maybe by having only one line with the column titles at the top, and the 3 dividers below would make it intuitive?
The collapsable feature of each subheader would still allow for an easier (or quicker) way for users to access the column titles to filter.

@jaimergp
Copy link
Member

I am not convinced this is The Right Way™️ so if you have ideas, I would like to hear them.

Maybe you can use https://docusaurus.io/docs/api/themes/configuration#use-color-mode?

@jaimergp
Copy link
Member

There are some conflicts because of #2093. I can help resolve them if you want, @afshin. Let me know and I'll push :)

@SylvainCorlay
Copy link
Member

There are some conflicts because of #2093. I can help resolve them if you want, @afshin. Let me know and I'll push :)

That would be great!

@jaimergp
Copy link
Member

jaimergp commented Mar 11, 2024

I am not convinced this is The Right Way™️ so if you have ideas, I would like to hear them.

Maybe you can use https://docusaurus.io/docs/api/themes/configuration#use-color-mode?

More dark mode visibility issues, but I think these can be mostly addressed with Gabriela's suggestion at #2090 (comment)

image

@afshin
Copy link
Member Author

afshin commented Mar 11, 2024

I am not convinced this is The Right Way™️ so if you have ideas, I would like to hear them.

Maybe you can use https://docusaurus.io/docs/api/themes/configuration#use-color-mode?

I think I can use this, yes. I'll give a try! Thanks @jaimergp!

And yes please, if merge conflicts arise that you know the right version of, please help! I'll make a comment in this thread when I have resolved the theme switching issue.

@afshin afshin force-pushed the status-dashboard branch from 3eeecfa to 92e2081 Compare March 11, 2024 12:36
@afshin
Copy link
Member Author

afshin commented Mar 11, 2024

The theme switching now uses the color mode helper. It still needs to – unfortunately – compute the style value of the CSS color variables once at runtime (which is also why there are two new CSS variables that are invariant and define light and dark primary colors and are used to populate the respective dark/light --ifm-color-primary variables). This seems unavoidable. But the rest of it is cleaner without having to use a mutation observer for each change since I can rely on useColorMode for this, instead. The CSS has an explanatory note:

:root {
  /*
    These two variables are defined in root and used at runtime for calculating
    the light mode and dark mode values of the primary color variable in order
    to populate `backgroundColor` prop `<UsageChart>`.
  */
  --ifm-color-primary-light-mode: #008478;
  --ifm-color-primary-dark-mode: #4db6ac;
  /* ... other stuff ... */
  --ifm-color-primary: var(--ifm-color-primary-light-mode);
  /* ... other stuff ... */
}

[data-theme="dark"] {
  --ifm-color-primary: var(--ifm-color-primary-dark-mode);
  /* ... other stuff ... */
}

Updating the colors of the details page, I would propose doing this as a follow-on.

@jaimergp
Copy link
Member

Thanks a bunch! Will merge by EOD.

@SylvainCorlay
Copy link
Member

:shipit:

@jaimergp jaimergp merged commit 6629163 into conda-forge:main Mar 11, 2024
5 checks passed
@jaimergp
Copy link
Member

It's live! https://conda-forge.org/status

@jakirkham
Copy link
Member

Looks like we lost the migration table's "Done" column (please see screenshot below). It is already in the details page

Adding it back in PR: #2114

Please let us know if anything else is needed

Screenshot 2024-03-11 at 2 09 20 PM

@h-vetinari
Copy link
Member

How long does it take for new migrations to get added to the status page, resp. must happen for it to be picked up? I cannot see a migration for libsentencepiece, 24h after merging conda-forge/conda-forge-pinning-feedstock#5644...

@jakirkham
Copy link
Member

jakirkham commented Mar 19, 2024

More likely this is a bot bug than a webpage bug

The webpage is just reading the bot's data

The migration referenced doesn't show up in the bot's data

Would suggest raising a new bot issue

@jaimergp jaimergp mentioned this pull request Mar 26, 2024
16 tasks
@h-vetinari
Copy link
Member

Can someone who was involved in this effort take a look at #2137 please? Looking at a big migration like python312 with >500 open PRs, it's impossible to tell where the biggest bottlenecks are currently. We need to know the number of total children, not just the immediate ones, otherwise shepherding such big migrations becomes basically impossible.

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

Successfully merging this pull request may close these issues.

9 participants