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

Grad classes async bug fix #2094

Merged
merged 2 commits into from
Nov 22, 2024
Merged

Conversation

mliu6723
Copy link
Contributor

@mliu6723 mliu6723 commented Oct 26, 2024

Summary

Updating fetching grad classes to be more consistent with how we fetch undergrad classes. Specifically, dealing with an async bug fix.

Changelog

  • [General] [Fix] - Fixed how we fetch grad classes to be consistent with how we fetch undergrad classes.

@morebytes
Copy link
Contributor

morebytes commented Nov 2, 2024

Examined the changed code; I don't think this was working before because it was trying to cast a Future to a String, which are completely unrelated and incompatible types. Combined with the fact that we explicitly ignore most exceptions at runtime, this would explain why it was silently failing. On a separate note, this is a good case study for the code style guide project...

@morebytes morebytes linked an issue Nov 2, 2024 that may be closed by this pull request
7 tasks
@morebytes morebytes removed a link to an issue Nov 21, 2024
7 tasks
@morebytes
Copy link
Contributor

Unlinked original issue from this PR, since it doesn't seem to fix the grad classes bug.

@mliu6723 mliu6723 changed the title Grad classes bug fix Grad classes async bug fix Nov 22, 2024
@mliu6723 mliu6723 self-assigned this Nov 22, 2024
Copy link
Contributor

@morebytes morebytes left a comment

Choose a reason for hiding this comment

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

Looks good, doesn't need testing given how small the change is

@morebytes morebytes merged commit c44413d into UCSD:experimental Nov 22, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

2 participants