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

Reduce warnings output in CI #7738

Closed
2 tasks
wochinge opened this issue Jan 15, 2021 · 7 comments
Closed
2 tasks

Reduce warnings output in CI #7738

wochinge opened this issue Jan 15, 2021 · 7 comments
Assignees
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework area:rasa-oss/infrastructure 🚅 All things related to infrastructure or deployments effort:atom-squad/2 Label which is used by the Rasa Atom squad to do internal estimation of task sizes. type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style.

Comments

@wochinge
Copy link
Contributor

wochinge commented Jan 15, 2021

Description of Problem:
When running our CI builds there are currently a lot of warnings in the output in the CI. This makes it harder to detect errors / problems in the builds and also makes us blind to new warnings caused by our changes (broken window theory).

Overview of the Solution:

  • I think there is a feature in pytest which will mark a test as failed if it emits an unhandled warning (see here)

Definition of Done:

  • all expected warnings are handled in the tests using pytest.warns
  • no unexpected warnings are emitted
@wochinge wochinge added type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style. area:rasa-oss 🎡 Anything related to the open source Rasa framework area:rasa-oss/infrastructure 🚅 All things related to infrastructure or deployments labels Jan 15, 2021
@wochinge
Copy link
Contributor Author

this issue is only supposed to handle warnings emitted by rasa directly. Warnings by used third-party packages should be handled separately.

@wochinge wochinge added the effort:atom-squad/2 Label which is used by the Rasa Atom squad to do internal estimation of task sizes. label Jan 22, 2021
@Imod7 Imod7 self-assigned this Feb 22, 2021
@Imod7
Copy link
Contributor

Imod7 commented Feb 26, 2021

Created a first doc with a summary of the most important (if not all) warnings from the CI logs and their number of occurrences (hence their impact in the output). From the summary, the warnings for which I did a first investigation have a link to their corresponding row in the sheet warnings_detail where more information is added (eg. the reason the warning is raised, the possible solution and the tests that can be run to replicate it).

@Imod7
Copy link
Contributor

Imod7 commented Mar 16, 2021

@RasaHQ/enable-squad I think I created quite a few sub Issues based on this analysis (warnings_summary sheet). I cannot say if it is exhaustive but I think it is a good start. Should I unassigned myself from this parent ticket and keep it open until all sub/linked-to-it tickets are closed? Or should I close it since we have a good amount of child Issues that are open and address this one? Based on the current definition of done of this ticket I cannot close it btw. Also, if you notice an issue that I am missing and I should also create or if there is an issue that is a duplicate, please let me know! 🙏🏽

@wochinge
Copy link
Contributor Author

Sounds good! How many warnings out of the total warnings are roughly covered by your created child issues?

@Imod7
Copy link
Contributor

Imod7 commented Mar 18, 2021

I would say 95% of all warnings are covered taking into account that :

  • I grouped correctly all warnings related to training_data.
  • I might missed some warnings during the analysis.

Worth mentioning that I downloaded a latest log file from the CI tests and I noticed that some warnings do not appear anymore (so I can close the corresponding ticket) but also there might be some new ones. Maybe it is worth it that I do another quick analysis on the latest logs after the last release to check where we are.

@wochinge
Copy link
Contributor Author

wochinge commented Mar 18, 2021

Ok, how about we do another sub issue which is Re-evaluate CI warnings after improvements and then close this one? Its goal would be to check how many warnings are remaining and what we should do about them.

@Imod7
Copy link
Contributor

Imod7 commented Mar 18, 2021

Yes, that is a great idea! 👌🏾 I created the new sub issue so I am now closing the current one.

@Imod7 Imod7 closed this as completed Mar 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rasa-oss 🎡 Anything related to the open source Rasa framework area:rasa-oss/infrastructure 🚅 All things related to infrastructure or deployments effort:atom-squad/2 Label which is used by the Rasa Atom squad to do internal estimation of task sizes. type:maintenance 🔧 Improvements to tooling, testing, deployments, infrastructure, code style.
Projects
None yet
Development

No branches or pull requests

3 participants