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.

Sign up for GitHub

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

Jump to bottom

New function not based on list endpoint #255

Merged
merged 1 commit into from
Aug 20, 2024
Merged

New function not based on list endpoint #255

merged 1 commit into from
Aug 20, 2024

Conversation

benjben
Copy link
Contributor

@benjben benjben commented Aug 7, 2024 •
edited
Loading

PDP-1358

@benjben benjben marked this pull request as draft August 7, 2024 14:37
@benjben benjben marked this pull request as ready for review August 9, 2024 10:38
@benjben benjben changed the title [WIP] new function not based on list endpoint New function not based on list endpoint Aug 9, 2024
@istreeter
Copy link
Contributor

Since you said

don't hesitate to propose an alternative

I came up with this. The tests don't all pass, so I've probably got something wrong. But hopefully you'll see what it's trying to do.

@benjben
Copy link
Contributor Author

benjben commented Aug 12, 2024

Thanks @istreeter ! Your implementation is much more simple, I'll go with it !
@pondzix I think that you are reviewing the PR, bear with me while I do the change, I also need to change the return type of the function for common-streams and I need to add a test that should have failed with Ian's change (they all pass).

@pondzix
Copy link
Contributor

pondzix commented Aug 12, 2024

@istreeter please, we don't use go here 😁

istreeter
istreeter reviewed Aug 13, 2024
View reviewed changes
Copy link
Contributor

@istreeter istreeter left a comment

Choose a reason for hiding this comment

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

Would be nice to see some scaladoc on the public-facing methods.

modules/core/src/main/scala/com.snowplowanalytics.iglu/client/resolver/Resolver.scala Outdated Show resolved Hide resolved
modules/core/src/main/scala/com.snowplowanalytics.iglu/client/resolver/Resolver.scala Outdated Show resolved Hide resolved
modules/core/src/main/scala/com.snowplowanalytics.iglu/client/resolver/Resolver.scala Outdated Show resolved Hide resolved
modules/core/src/main/scala/com.snowplowanalytics.iglu/client/resolver/Resolver.scala Outdated Show resolved Hide resolved
@benjben benjben requested a review from istreeter August 19, 2024 09:48
@benjben benjben mentioned this pull request Aug 19, 2024
Merged
@benjben benjben requested a review from istreeter August 20, 2024 13:20
@benjben benjben merged commit 904ca1f into develop Aug 20, 2024
1 check passed
@benjben benjben deleted the not_use_list branch August 20, 2024 14:14
istreeter added a commit to snowplow-incubator/snowplow-lake-loader that referenced this pull request Sep 20, 2024
@istreeter
Upgrade common-streams 0.8.0-M4
854ef98 
The following improvements are introduced via common-streams 0.8.0-M4:

- Fields starting with a digit are now prefixed with an underscore `_`.
  This is needed for Hudi, which does not allow fields starting with a
  digit (snowplow/schema-ddl#209)
- New kinesis source implementation without fs2-kinesis
  (snowplow-incubator/common-streams#84)
- Iglu schemas are resolved in parallel, for short pause times during
  event processing (snowplow-incubator/common-streams#85)
- Common http client configured with restricted max connections per
  server (snowplow-incubator/common-streams#87)
- Iglu scala client 3.2.0 no longer relies on the "list" schemas
  endpoint (snowplow/iglu-scala-client#255)
@istreeter istreeter mentioned this pull request Sep 20, 2024
Merged
istreeter added a commit to snowplow-incubator/snowplow-lake-loader that referenced this pull request Sep 20, 2024
@istreeter
Upgrade common-streams 0.8.0-M4 (#81)
ebb087e 
The following improvements are introduced via common-streams 0.8.0-M4:

- Fields starting with a digit are now prefixed with an underscore `_`.
  This is needed for Hudi, which does not allow fields starting with a
  digit (snowplow/schema-ddl#209)
- New kinesis source implementation without fs2-kinesis
  (snowplow-incubator/common-streams#84)
- Iglu schemas are resolved in parallel, for short pause times during
  event processing (snowplow-incubator/common-streams#85)
- Common http client configured with restricted max connections per
  server (snowplow-incubator/common-streams#87)
- Iglu scala client 3.2.0 no longer relies on the "list" schemas
  endpoint (snowplow/iglu-scala-client#255)
oguzhanunlu pushed a commit to snowplow-incubator/snowplow-lake-loader that referenced this pull request Nov 1, 2024
@istreeter @oguzhanunlu
Upgrade common-streams 0.8.0-M4 (#81)
6b6fb89 
The following improvements are introduced via common-streams 0.8.0-M4:

- Fields starting with a digit are now prefixed with an underscore `_`.
  This is needed for Hudi, which does not allow fields starting with a
  digit (snowplow/schema-ddl#209)
- New kinesis source implementation without fs2-kinesis
  (snowplow-incubator/common-streams#84)
- Iglu schemas are resolved in parallel, for short pause times during
  event processing (snowplow-incubator/common-streams#85)
- Common http client configured with restricted max connections per
  server (snowplow-incubator/common-streams#87)
- Iglu scala client 3.2.0 no longer relies on the "list" schemas
  endpoint (snowplow/iglu-scala-client#255)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pondzix pondzix pondzix left review comments

@istreeter istreeter istreeter approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benjben @istreeter @pondzix