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

Bump pydantic #646

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Bump pydantic #646

wants to merge 2 commits into from

Conversation

shawaj
Copy link
Member

@shawaj shawaj commented Jul 9, 2023

Closes: #644

Issue

  • Link:
  • Summary:

How

Screenshots

References

Checklist

  • Tests added
  • Cleaned up commit history (rebase!)
  • Documentation added
  • Thought about variable and method names

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@shawaj shawaj marked this pull request as ready for review July 9, 2023 22:52
@shawaj shawaj requested review from a team as code owners July 9, 2023 22:52
@shawaj
Copy link
Member Author

shawaj commented Jul 9, 2023

@pritamghanghas since you originally worked on this(#404 #420 ), could you take a look?

Do we need to bump the diag schema and the relevant python files to include all of the new information available in the diagnostics JSON? Or this is not necessary?

@shawaj shawaj requested a review from pritamghanghas July 9, 2023 23:00
@pritamghanghas
Copy link
Contributor

I don't think anyone is using it. This was to upload the data to gcp bucket that triggers a function, that puts it in big query. I would say just retire it and save billing.

@pritamghanghas
Copy link
Contributor

pritamghanghas commented Jul 10, 2023

@KevinWassermann94 would know more if anyone is using the big query data for analysis. We have to keep in mind if more freeloaders join our fleets this will be unnecessary cost bump.

@pritamghanghas
Copy link
Contributor

may be we should just change perform_hw_diagnostics(ship=True) to perform_hw_diagnostics(ship=False) in app.py and be done with it.

@shawaj
Copy link
Member Author

shawaj commented Jul 10, 2023

@pritamghanghas Kevin says it's costing next to nothing and we do sometimes use it, albeit infrequently.

Do I need to modify it to actually provide all the data that's in the json
JSON? Or how does it work?

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.

Issues with pydantic 2.0.2
2 participants