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

Dependencies: Update to lorrystream 0.0.4 and commons-codec 0.0.6. Adjust code. Add software tests. #224

Merged
merged 5 commits into from
Aug 16, 2024

Conversation

amotl
Copy link
Member

@amotl amotl commented Aug 16, 2024

Problem

On the last occasion, dependencies to lorrystream have not been verified correctly, so it was expectable that things go south at some point. Pinning to a branch, or pinning to a specific commit, are both not satisfying solutions to compensate properly for aligning things that are in flux by nature. C'est la vie.

Solution

This patch fixes them, and adds rudimentary software tests to avoid tripping into the same situation as before.

References

/cc @wierdvanderhaar, @surister, @ckurze

@amotl amotl requested review from hammerhead, surister, hlcianfagna and wierdvanderhaar and removed request for surister and wierdvanderhaar August 16, 2024 10:37
On the last occasion, dependencies to lorrystream have not been verified
correctly, so it was expectable that things go south.

This patch fixes them, and adds rudimentary software tests to avoid
tripping into the same situation as before.
@amotl amotl force-pushed the fix-lorry-2 branch 2 times, most recently from 564a668 to ad344d2 Compare August 16, 2024 10:53
Copy link

@hlcianfagna hlcianfagna left a comment

Choose a reason for hiding this comment

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

This builds and now I can succesfully

from cratedb_toolkit.iac.aws import DynamoDBKinesisPipe, LambdaFactory, LambdaPythonImage

but I cannot use the new commons-codec with it

lorrystream 0.0.3 requires commons-codec==0.0.3, but you have commons-codec 0.0.5 which is incompatible

@amotl
Copy link
Member Author

amotl commented Aug 16, 2024

Thanks for reporting.

It is probably similar to that spot reported by CI:

That other patch relaxes dependency constraints a bit. It will hopefully improve situations like that.

@amotl amotl changed the title Dependencies: Adjust code for lorrystream version 0.0.3. Add tests. Dependencies: Update to lorrystream 0.0.4 and commons-codec 0.0.6. Adjust code. Add software tests. Aug 16, 2024
@amotl amotl force-pushed the fix-lorry-2 branch 2 times, most recently from 3ed3552 to 794b832 Compare August 16, 2024 14:51
@amotl amotl requested review from hlcianfagna and surister August 16, 2024 14:54
@amotl amotl marked this pull request as ready for review August 16, 2024 14:55
amotl added 4 commits August 16, 2024 19:09
It provides some woes around the `multiprocess` package, becoming
apparent when building the OCI image. So, let's just not include it into
the standard OCI release package for now.
The admonition raised by GHA check warnings is:

> Legacy key/value format with whitespace separator should not be used
>
> LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy
> "ENV key value" format
> More info: https://docs.docker.com/go/dockerfile/rule/legacy-key-value-format/
@amotl amotl merged commit a5ab31f into main Aug 16, 2024
27 checks passed
@amotl amotl deleted the fix-lorry-2 branch August 16, 2024 17:15
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