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

Add logger::log_shiny_input_changes once it allows to log to a custom namespace #239

Closed
Tracked by #74
kpagacz opened this issue Nov 9, 2021 · 10 comments · Fixed by #750
Closed
Tracked by #74

Add logger::log_shiny_input_changes once it allows to log to a custom namespace #239

kpagacz opened this issue Nov 9, 2021 · 10 comments · Fixed by #750
Assignees
Labels
core enhancement New feature or request
Milestone

Comments

@kpagacz
Copy link
Contributor

kpagacz commented Nov 9, 2021

The current, latest version of logger does not allow logging log_shiny_input_changes to a custom namespace.

We have a couple of options:

The objective of the issue is to add the functionality (either our own or logger's) to log changes in shiny inputs. Make sure to exclude sensitive shiny inputs.

@kpagacz
Copy link
Contributor Author

kpagacz commented Nov 23, 2021

The guys still didn't answer our PR requests. What is the purpose of this task then?

@pawelru
Copy link
Contributor

pawelru commented Jul 20, 2022

Alternatively we can use shinylogs package but we need to research the connection to the logger

@donyunardi
Copy link
Contributor

The PR that @kpagacz created already merged so this issue is no longer a blocker.
I did play around with this:

image

We can pick this up after the ddl refactor.

@donyunardi
Copy link
Contributor

donyunardi commented Oct 26, 2023

Note: In the example above, I was adding the log at the teal level, not the teal.module.xxx

@pawelru
Copy link
Contributor

pawelru commented Oct 27, 2023

@donyunardi the PR is there on master but I am not too sure if this got released on CRAN. We should use CRAN only dependencies. This guy is not tagging on his repo so it's hard to check it from there. Using github.com/cran/logger I double checked that the latest release (which is 2 years old btw) does not include the fix.

@m7pr
Copy link
Contributor

m7pr commented Feb 29, 2024

logger wasn't released still. it's 820 days already daroczig/logger#93 (comment)

@m7pr
Copy link
Contributor

m7pr commented Feb 29, 2024

After a ping he decided to do a release at the end of this week. daroczig/logger#144

@m7pr
Copy link
Contributor

m7pr commented Mar 7, 2024

Hey during this card daroczig/logger#144 logger was released on CRAN. This is no longer blocked
image

@pawelru
Copy link
Contributor

pawelru commented May 13, 2024

hey @m7pr I noticed you have done it for modules only. I think we are also interested in covering the "core" as well - teal, teal.slice and others if applicable. Otherwise this feels to be incomplete.

@m7pr
Copy link
Contributor

m7pr commented May 13, 2024

yeah, the scope was ambiguous. thanks for the clarification

@m7pr m7pr closed this as completed in #750 May 16, 2024
m7pr added a commit that referenced this issue May 16, 2024
Close #239 

The example loading for `tm_a_pca` on a startup shows how the input
change logging is made.

We are connected to package namespace (`namespace =
"teal.modules.general"`)
that is created on a startup with
`teal.logger::register_logger(namespace = "teal.modules.general")`

This function needs to be called in the server function, so there is no
way this can be run during the package startup.

<img width="931" alt="image"
src="https://github.com/insightsengineering/teal.modules.general/assets/133694481/e9480872-4fd0-4c97-8b59-fdeba8fd41a2">

**Note:** Needed to bring `logger` to Imports.

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
m7pr added a commit to insightsengineering/teal.slice that referenced this issue May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants