Skip to content
This repository was archived by the owner on Jan 21, 2024. It is now read-only.

Use custom deserializer and apply masking in place #29

Open
wants to merge 12 commits into
base: mule-4.x
Choose a base branch
from

Conversation

cut-to-the-cheese
Copy link

@cut-to-the-cheese cut-to-the-cheese commented May 11, 2021

Fix for: #28

this.enabled = enabled;

blacklistedKeys = new HashSet<>();
blacklistedJsonPaths = new HashSet<>();
Copy link
Author

Choose a reason for hiding this comment

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

No need for a path check. For one it is case sensitive, and secondly the same task can be accomplished using just the keys.

@GeraldLoeffler
Copy link

@jahanzebbaber : do you have write/maintain/admin permissions on this repo? I thought that this repo is now orphaned since @skuller no longer maintains it?

@cut-to-the-cheese
Copy link
Author

@GeraldLoeffler I do not. We might end up just making this change to our internal GitHub organization if it is orphaned.

@skuller
Copy link
Contributor

skuller commented May 14, 2021

@GeraldLoeffler @jahanzebbaber please reach out to @juancavallotti and @DavoCoder for ongoing requests!

@GeraldLoeffler
Copy link

@skuller : please relax: there are no requests to you!

@GeraldLoeffler
Copy link

GeraldLoeffler commented May 17, 2021

@jahanzebbaber : in that case (you not having write permissions to this repo) can i please suggest that you open your Issues/PRs against this repo instead?: https://github.com/mulesoft-consulting/training-json-logger . This is a "fork" (not technically true/possible, because it's in the same GitHub org) of this repo and we have CI/CD pipelines in place to publish the JSON logger to the MuleSoft Training Exchange. We are using JSON logger in 2 of our courses, and plan to maintain our fork and the Exchange artifacts for the foreseeable future. So we're happy to merge your PRs over there if they work...

@cut-to-the-cheese
Copy link
Author

@GeraldLoeffler thank you for the suggestion. I can raise this PR as is against the repo you mentioned. In its current state it fixes the performance bottleneck mentioned in #28. But if this repo is orphaned, I will probably remove everything that is not being used by our teams.

@GeraldLoeffler
Copy link

thanks @jahanzebbaber , looking forward to your PR on our repo. As i said, our fork of the JSON logger is heavily used by our students and you're happy to use it too:

<dependency>
  <groupId>560e2d13-1898-4228-95d0-5a05ec751d10</groupId>
  <artifactId>json-logger</artifactId>
  <version>2.0.1</version>
  <classifier>mule-plugin</classifier>
</dependency>
...
<repository>
  <!-- Anypoint Exchange repository of the MuleSoft Training Anypoint Platform organization, requires authentication -->
  <id>anypoint-exchange-560e2d13-1898-4228-95d0-5a05ec751d10</id>
  <name>MuleSoft Training Anypoint Exchange</name>
  <url>https://maven.anypoint.mulesoft.com/api/v2/maven</url>
</repository>

and

<server>
  <!-- Anypoint Exchange repository of the MuleSoft Training Anypoint Platform organization -->
  <id>anypoint-exchange-560e2d13-1898-4228-95d0-5a05ec751d10</id>
  <username>training-devl2-public-exchange-viewer</username>
  <password>PublicTra1n1ngExchanger</password>
</server>

@cut-to-the-cheese
Copy link
Author

@GeraldLoeffler I just got around to raising the PR today. But I am getting a 404 while hitting https://github.com/mulesoft-consulting/training-json-logger. Thank you

@GeraldLoeffler
Copy link

@GeraldLoeffler I just got around to raising the PR today. But I am getting a 404 while hitting https://github.com/mulesoft-consulting/training-json-logger. Thank you

thank you very much @jahanzebbaber . You need to be authenticated against GitHub and in the https://github.com/orgs/mulesoft-consulting/teams/employees GitHub Team to be able to see https://github.com/mulesoft-consulting/training-json-logger . Are you? - every employee should be...

@cut-to-the-cheese
Copy link
Author

@GeraldLoeffler I am not a Mulesoft employee. Sorry for the confusion.

@GeraldLoeffler
Copy link

@GeraldLoeffler I am not a Mulesoft employee. Sorry for the confusion.

hey @jahanzebbaber : i've added you individually to that repo, you have Triage access now. Wouldn't want to miss your great contribution! Sorry for the back-and-forth ;)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants