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

Upgrade common requirements: Utils #7730

Merged

Conversation

EvaBardou
Copy link
Collaborator

@EvaBardou EvaBardou commented Jun 28, 2023

⚠️ Please review and test thoroughly before merging, I'm not really familiar with Treeherder and its features, I may have missed some broken behaviors while testing the upgrade.

What is left to do:

Comments on various packages

newrelic

  • Replaced record_exception with notice_error as requested here
  • Replaced add_custom_parameter with add_custom_attribute as requested here

mozlog

I've bump the package but I didn't find any release notes...

importlib-metadata

Removed because it was unused in the repository code

typed-ast

Removed because it was unused in the repository code

mozci

AttributeError("module 'taskcluster.aio' has no attribute 'Queue'")

coreapi

CoreAPI auto schema which was used to serve the DRF API documentation has been replaced by OpenAPI. This new schema has to be served using a custom UI, here we picked ReDoc. See DRF documentation for more information.

There's definitely a certain amount of work to be done on the schema construction to make the new UI more pleasant to read and navigate.

In the meantime, API documentation is still available at the URL /docs/, while the new schema is available at /openapi/ (it was /docs/schema.js before).

@EvaBardou EvaBardou marked this pull request as draft June 28, 2023 13:14
@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2023

Codecov Report

Patch coverage: 68.42% and no project coverage change.

Comparison is base (c457ccc) 76.74% compared to head (a9a44c2) 76.74%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7730   +/-   ##
=======================================
  Coverage   76.74%   76.74%           
=======================================
  Files         535      535           
  Lines       26329    26331    +2     
  Branches     3326     3326           
=======================================
+ Hits        20205    20207    +2     
  Misses       5959     5959           
  Partials      165      165           
Impacted Files Coverage Δ
tests/push_health/test_compare.py 100.00% <ø> (ø)
tests/test_middleware.py 100.00% <ø> (ø)
treeherder/etl/jobs.py 94.77% <0.00%> (ø)
treeherder/etl/pushlog.py 75.38% <0.00%> (ø)
treeherder/etl/tasks/pulse_tasks.py 50.00% <0.00%> (ø)
treeherder/etl/tasks/pushlog_tasks.py 61.53% <0.00%> (ø)
treeherder/model/models.py 82.07% <0.00%> (ø)
treeherder/log_parser/tasks.py 63.51% <66.66%> (ø)
treeherder/middleware.py 95.00% <83.33%> (+0.26%) ⬆️
treeherder/config/settings.py 88.52% <100.00%> (+0.19%) ⬆️
... and 11 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@La0 La0 left a comment

Choose a reason for hiding this comment

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

Tested on our locally shared instance, no issue with prod dump 👌

The whitenoise update does not seem too bad all things considered.

@EvaBardou EvaBardou marked this pull request as ready for review July 3, 2023 13:10
@EvaBardou
Copy link
Collaborator Author

@Archaeopteryx This is ready to be tested and reviewed whenever you are 😄

@vrigal vrigal mentioned this pull request Jul 5, 2023
@Archaeopteryx Archaeopteryx merged commit a6d7768 into mozilla:master Jul 6, 2023
1 check passed
@Archaeopteryx
Copy link
Collaborator

  1. The deployment of these changes broke Treeherder: The resource at https://treeherder.mozilla.org/assets/index.0b2df09f.js fails with an internal server error (502 - Bad Gateway). Both production and staging were affected and the previous working commit has been redeployed.
  2. After discussion with the security engineers, the resources for the redoc page should be vendored and served from the Treeherder server and be loaded from external resources.
    @EvaBardou

@Archaeopteryx
Copy link
Collaborator

The file is listed as generated among the js bundle files in the log. There don't seem to logs which logs the individual files getting deployed.
Please reach out if something should be deployed to the prototype instance for testing, e.g. a deployment without the whitenoise upgrade.

@EvaBardou
Copy link
Collaborator Author

@Archaeopteryx Hi, I'm sorry the deployment didn't work in production, I'll take the time to look at what's wrong with this next week.

In the meantime and as a preventive measure, I'm going to submit a PR that restores WhiteNoise and Redoc to their previous versions to recover an unbroken state on master.

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