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

Fix some warnings and add command line parsing for unordered field keys #330

Merged
merged 2 commits into from
Oct 14, 2020

Conversation

idreeskhan
Copy link
Contributor

Don't know if this is a good idea or not but it was annoying to me to have to import ratatool as a dependency just to use this feature (and therefore commit diff jobs into a pipeline repo). Basically allows -> demarcated field key mappings for nested records that can be ',' delimited to specify multiple field mappings. Let me know what you think

@codecov
Copy link

codecov bot commented Oct 6, 2020

Codecov Report

Merging #330 into master will increase coverage by 0.04%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #330      +/-   ##
==========================================
+ Coverage   71.06%   71.10%   +0.04%     
==========================================
  Files          35       35              
  Lines        1441     1450       +9     
  Branches      115      120       +5     
==========================================
+ Hits         1024     1031       +7     
- Misses        417      419       +2     
Flag Coverage Δ
#ratatoolCli 3.16% <0.00%> (-0.03%) ⬇️
#ratatoolCommon 100.00% <ø> (ø)
#ratatoolDiffy 30.61% <66.66%> (+0.35%) ⬆️
#ratatoolExamples 18.79% <0.00%> (-0.13%) ⬇️
#ratatoolSampling 62.99% <ø> (ø)
#ratatoolScalacheck 81.81% <ø> (ø)
#ratatoolShapeless 5.25% <0.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ala/com/spotify/ratatool/samplers/BigSampler.scala 78.31% <ø> (ø)
...ol/samplers/util/SamplerSCollectionFunctions.scala 92.79% <ø> (ø)
...in/scala/com/spotify/ratatool/diffy/BigDiffy.scala 55.81% <66.66%> (+1.21%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be6b50c...016e7d6. Read the comment docs.

@idreeskhan
Copy link
Contributor Author

This fixes #103

| --lhs=<path> LHS File path or BigQuery table
| --rhs=<path> RHS File path or BigQuery table
| --output=<output> File path prefix for output
| --ignore=<keys> ',' separated field list to ignore
| --unordered=<keys> ',' separated field list to treat as unordered
| --unorderedFieldKey=<key> ',' separated list of keys for fields which are unordered nested records. Mappings use '->'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking of switching this to : delimited

Copy link
Contributor

Choose a reason for hiding this comment

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

I like : better than ->

Copy link
Contributor

@anne-decusatis anne-decusatis left a comment

Choose a reason for hiding this comment

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

just to confirm, this was already available in scala, we're just adding to CLI? if so, looks good, though I prefer the different delimiter

| --lhs=<path> LHS File path or BigQuery table
| --rhs=<path> RHS File path or BigQuery table
| --output=<output> File path prefix for output
| --ignore=<keys> ',' separated field list to ignore
| --unordered=<keys> ',' separated field list to treat as unordered
| --unorderedFieldKey=<key> ',' separated list of keys for fields which are unordered nested records. Mappings use '->'
Copy link
Contributor

Choose a reason for hiding this comment

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

I like : better than ->

@@ -240,6 +242,15 @@ class BigDiffyTest extends PipelineSpec {
keyValues.toString shouldBe "foo_bar"
}

"BigDiffy unorderedKeysMap" should "work with multiple unordered keys" in {
val keyMappings = List("record.nested_record->key", "record.other_nested_record->other_key")
val unorderdKeys = BigDiffy.unorderedKeysMap(keyMappings)
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: typo, unordered

@idreeskhan idreeskhan merged commit e40b567 into master Oct 14, 2020
@idreeskhan idreeskhan deleted the idrees/unordered-field-keys branch October 14, 2020 16:53
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.

2 participants