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 decode option to redact line sequence data from CSV #123

Conversation

taylor-redden-papa
Copy link
Contributor

@taylor-redden-papa taylor-redden-papa commented Sep 7, 2023

This PR addresses
CSVs might contain sensitive data, so we could allow a decode option to not include that data as part of the exception messaging.

Open questions
It could be argued that this isn't needed at all and apps using this library could just not log this out when they get this exception. But for long standing apps, it might be easier for them to add this decode option in the few areas where they need to without changing any other telemetry or logging.

The naming from the option was inspired by the ecto field option redact. I thought this might be a similar situation.

Lastly, I notice we are just adding another parameter to all the functions here. This is outside the scope of this PR, but would it possibly be better to pass around the options starting at create_row_transform, and then extract them when needed down the line?

Checklist

  • Tests added ✅
  • Coverage increases or stays the same
  • Docs updated ✅
  • Changelog updated

@taylor-redden-papa
Copy link
Contributor Author

I just realized I think there is a way easier way to do this...

@taylor-redden-papa
Copy link
Contributor Author

see #124

@taylor-redden-papa taylor-redden-papa deleted the redact-exception branch September 7, 2023 22:05
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.

1 participant