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

#1167 refactor to use FP paradigm for schema validation logic #1185

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

junaidzm13
Copy link
Contributor

No description provided.

Copy link

netlify bot commented Feb 9, 2024

Deploy Preview for papaya-valkyrie-395400 canceled.

Name Link
🔨 Latest commit 73593a9
🔍 Latest deploy log https://app.netlify.com/sites/papaya-valkyrie-395400/deploys/65cad75c8bf6c900085e6436

@junaidzm13 junaidzm13 marked this pull request as ready for review February 9, 2024 06:13
@junaidzm13 junaidzm13 force-pushed the 1167-follow-up-small-refactor branch 2 times, most recently from 03f4943 to e80e66a Compare February 9, 2024 06:20

None
externalFields
.find(field => externalSchema.schemaFields.forall(_.name != field))
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason to switch to forall rather than using exists?
Exists will exit early on first element it find that is a match rather than traversing the whole collection
https://stackoverflow.com/questions/59922992/when-true-does-scala-exists-stop-at-the-first-element-that-satisfies-the-predic

Copy link
Contributor Author

@junaidzm13 junaidzm13 Feb 13, 2024

Choose a reason for hiding this comment

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

forall exits early as well, so they're same in that aspect. I just find forall easier to follow in this scenario.

@junaidzm13 junaidzm13 force-pushed the 1167-follow-up-small-refactor branch from e80e66a to a1fc13d Compare February 13, 2024 02:40
- IgniteEntitySchema's invalid index exception to check for and include
  all invalid indexes
- SchemaMapper validation to not check for missing columns in fields
  map since there's a use-case (basket constituents) where it makes
  sense to not specify all internal columns in the fields map.
@junaidzm13 junaidzm13 force-pushed the 1167-follow-up-small-refactor branch from a1fc13d to 73593a9 Compare February 13, 2024 02:43
@naleeha naleeha self-requested a review February 13, 2024 11:53
@chrisjstevo chrisjstevo merged commit a5cbd1e into finos:main Feb 13, 2024
13 checks passed
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.

3 participants