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

order of function call corrected. On similar lines of rasa-sdk #13023

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

Conversation

gajendrarao1979
Copy link

Proposed changes:

  • ...

Status (please check what you already did):

  • added some tests for the functionality
  • updated the documentation
  • [*] updated the changelog (please check changelog for instructions)
  • [*] reformat files using black (please check Readme for instructions)

@gajendrarao1979 gajendrarao1979 requested a review from a team as a code owner March 13, 2024 07:47
@CLAassistant
Copy link

CLAassistant commented Mar 13, 2024

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@vcidst
Copy link
Contributor

vcidst commented Mar 25, 2024

Hi @gajendrarao1979, thank you for the pull request! Can you please describe the changes you've made and why are they required?

@gajendrarao1979
Copy link
Author

gajendrarao1979 commented Apr 16, 2024

@vcidst
Currently in rasa main.py the configure_logging_and_warnings function which configures the logging from the config file is called at line 103 and after that configure_colored_logging is called at line 116.

The configure_colored_logging function leverages coloredlogs module which discards and reinitializes logging.StreamHandler. Essentially this overwrites any changes coming from logging.yml (if someone is using StreamHandler).

In my PR I have updated the order of the function call to ensure that logging.yml changes are not overwritten. This is already correct in rasa-sdk code.

I hope this provides the explanation.

image

@gajendrarao1979
Copy link
Author

@vcidst Any updates on this?

@pdelagrave
Copy link

Necessary change for using on a production system where observability matters. Hoping to see this PR merged soon!

@vcidst
Copy link
Contributor

vcidst commented Sep 27, 2024

Hey @gajendrarao1979, we won't be able to merge these PRs as this repository isn't under active maintenance anymore. Please see this page for our product releases and maintenance policy

@arththebird
Copy link

arththebird commented Sep 30, 2024

Hey @gajendrarao1979, we won't be able to merge these PRs as this repository isn't under active maintenance anymore. Please see this page for our product releases and maintenance policy

Hi @vcidst, could you please clarify why this PR won't be merged? I've read the maintenance policy page you referenced and I can't see any mention of Rasa Open Source being deprecated. Is there an announcement from Rasa anywhere that the open source version is no longer maintained?

Thanks!

@gajendrarao1979
Copy link
Author

Hey @gajendrarao1979, we won't be able to merge these PRs as this repository isn't under active maintenance anymore. Please see this page for our product releases and maintenance policy

Hi @vcidst, could you please clarify why this PR won't be merged? I've read the maintenance policy page you referenced and I can't see any mention of Rasa Open Source being deprecated. Is there an announcement from Rasa anywhere that the open source version is no longer maintained?

Thanks!

@vcidst can you please comment on this? If RASA Opensource is no longer maintained then we need to find alternate solution.

@gajendrarao1979
Copy link
Author

@arththebird , @pdelagrave One workaround which we did to circumvent this issues is re-initialized logging in custom-channel code which gets executed after rasa initialization (not ideal but works until the fix in the library is available.)

image

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.

5 participants