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

Upstream changes #65

Merged
merged 12 commits into from
Jul 19, 2024
Merged

Upstream changes #65

merged 12 commits into from
Jul 19, 2024

Conversation

jem-davies
Copy link
Collaborator

@jem-davies jem-davies commented Jul 3, 2024 •
edited
Loading

  • Field max_retries added to the retry processor.
  • Algorithm fnv32 added to the hash bloblang method
  • Parameter escape_html added to the format_json() Bloblang method.
  • New array bloblang method
  • Go API: New generic key/value store methods added to the *Resources type.
  • Use new rickb777/period library for parse_duration_iso8601

@gregfurman
Copy link
Collaborator

Hey. Haven't given a proper look at changes yet but If possible, maybe remove or rename the merge commits 84093c7 and 524991e.

gregfurman
gregfurman requested changes Jul 5, 2024
View reviewed changes
Copy link
Collaborator

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

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

Good stuff, @jem-davies -- no doubt this was a gnarly one to work through. A couple of things:

  • Some upstream naming changes
  • Commits being added should be cleaned up a bit
  • Question re: new introduced max_retry config

internal/manager/mock/manager.go Outdated Show resolved Hide resolved
internal/config/test/case.go Outdated Show resolved Hide resolved
internal/manager/input_wrapper.go Outdated Show resolved Hide resolved
internal/bloblang/query/methods_strings.go Outdated Show resolved Hide resolved
internal/impl/pure/processor_retry.go Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
@jem-davies
Tighten up stream integration tests
c7462c0 
Reject messages which contain errors when running the integration
test suite.

This comes in handy when one wants to attach some processors to
the input(s) in integration tests for performing additional
validations such as throwing an error if some expected metadata
field isn't present or has the wrong value.

Signed-off-by: Mihai Todor <[email protected]>
Signed-off-by: Jem Davies <[email protected]>
@gregfurman
Copy link
Collaborator

Couple of things:

  • Can you run a git rebase and squash/reword commits where possible?
  • I see some commits are unverified so make sure all is signed. Lmk if you need help with that because I know rebase + signing is a mission.
  • I know it's a tall ask but can you try summarise the changes introduced in the PR description? I'd check in the CHANGELOG.old.md (should be renamed to CHANGELOG.upstream.md imo) and see what entries are relevant. It could even be worth cherry-picking from releases instead of from main...

@jem-davies jem-davies force-pushed the upstream-changes branch 3 times, most recently from 3fb6644 to 3497b83 Compare July 13, 2024 14:44
@jem-davies jem-davies marked this pull request as ready for review July 13, 2024 15:41
gregfurman
gregfurman requested changes Jul 14, 2024
View reviewed changes
internal/bloblang/query/methods_test.go Outdated Show resolved Hide resolved
internal/component/connection.go Outdated Show resolved Hide resolved
internal/bloblang/query/methods_strings.go Outdated Show resolved Hide resolved
internal/impl/pure/processor_retry.go Show resolved Hide resolved
public/service/processor.go Outdated Show resolved Hide resolved
website/docs/guides/bloblang/methods.md Show resolved Hide resolved
@gregfurman
Copy link
Collaborator

gregfurman commented Jul 14, 2024 •
edited
Loading

Looking good, dude! Please update CHANGELOG.old.md and CHANGELOG.md with relevant changes 😄 i.e

So in CHANGELOG.md add:

## 1.2.0 - TBD

### Upstream Changes Synced

- [v4.??.?? - 2024-??-??](./CHANGELOG.old.md#4.??.0-2024-??-??)

gregfurman
gregfurman reviewed Jul 19, 2024
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
jem-davies and others added 6 commits July 19, 2024 11:56
@jem-davies
Add fnv32 hash option
990300b 
Signed-off-by: Jem Davies <[email protected]>
@jem-davies @Jeffail
Fix docs typo in Processor.Process()
02fa7c3 
Also in `BatchProcessor.ProcessBatch()`.

According to Ash:

> You need to ensure that if you end up yielding N messages then
> you need to copy the original message N times rather than create
> N new instantiations. That's because we use the context from the
> original message to carry things like distributed tracing info
> and synchronous response mechanisms.

Co-authored-by: Ashley Jeffs <[email protected]>
Signed-off-by: Mihai Todor <[email protected]>
Signed-off-by: Jem Davies <[email protected]>
@jem-davies
Use new rickb777/period library for parse_duration_iso8601()
1498721 
This library supersedes `rickb777/date` for durations as
mentioned here: rickb777/period#1.

The maintainer advised to switch to this new library in several
other issues:
- rickb777/date#15
- rickb777/date#16
- rickb777/date#12

Signed-off-by: Mihai Todor <[email protected]>
Signed-off-by: Jem Davies <[email protected]>
@jem-davies
Expand connection status for inputs
f9faa39 
Signed-off-by: Jem Davies <[email protected]>
@jem-davies
Add output connection status
8a4751e 
Signed-off-by: Jem Davies <[email protected]>
@jem-davies
update changelogs
3d270c5 
Signed-off-by: Jem Davies <[email protected]>
gregfurman
gregfurman approved these changes Jul 19, 2024
View reviewed changes
Copy link
Collaborator

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

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

LGTM 💯 Nice job, dude. Some exciting incoming changes!

@jem-davies jem-davies merged commit 17147d6 into main Jul 19, 2024
3 checks passed
@jem-davies jem-davies deleted the upstream-changes branch July 19, 2024 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregfurman gregfurman gregfurman approved these changes

Assignees

@jem-davies jem-davies

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jem-davies @gregfurman