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

octo-logger-cpp: update checksum for 1.12.0 #24429

Closed
wants to merge 1 commit into from

Conversation

toge
Copy link
Contributor

@toge toge commented Jun 24, 2024

Summary

Changes to recipe: octo-logger-cpp/1.12.0

Motivation

tar ball of 1.12.0 is updated.

Details


@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 1 (718287b98e9dffbe7604120f7111c9d5a2a6a405):

  • octo-logger-cpp/1.12.0:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.5.0:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.9.0:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.4.0:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.6.1:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.2.0:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.11.0:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.1.0:
    All packages built successfully! (All logs)


Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 1 (718287b98e9dffbe7604120f7111c9d5a2a6a405):

  • octo-logger-cpp/1.12.0:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.11.0:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.2.0:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.9.0:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.6.1:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.5.0:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.4.0:
    All packages built successfully! (All logs)

  • octo-logger-cpp/1.1.0:
    All packages built successfully! (All logs)

Copy link
Contributor

@jcar87 jcar87 left a comment

Choose a reason for hiding this comment

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

Thanks @toge!

Leaving this pending internal verification - hopefully we are able to compare the changes against a backed up copy of the original file contents.

Please note that we will not accept checksum changes without verifying what changed - if we did, we may as well not use checksums at all!

Things to look out for:

  • it could be a malicious change if the account of the maintainers is compromised
  • it could be a benign change that causes breakages, for example:
    • it's source incompatible with the previously published version
    • downstream consumers are binary incompatible - a big problem when consuming the shared library (we would have two of the same version, but they wouldn;'t be compatible and the package_id mechanisms can't reflect this)

Copy link
Member

@AbrilRBS AbrilRBS left a comment

Choose a reason for hiding this comment

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

Changed code

diff -bur old new
diff --color=auto -bur old/src/aws/cloudwatch-sink.cpp new/src/aws/cloudwatch-sink.cpp
--- old/src/aws/cloudwatch-sink.cpp	2024-06-13 08:08:09
+++ new/src/aws/cloudwatch-sink.cpp	2024-06-16 08:04:14
@@ -83,6 +83,22 @@
     if (std::find(existing_log_streams_.begin(), existing_log_streams_.end(), log_stream_name) ==
         existing_log_streams_.end())
     {
+        Aws::CloudWatchLogs::Model::DescribeLogStreamsRequest describe_log_streams;
+        describe_log_streams.SetLogGroupName(log_group_name_.c_str());
+        describe_log_streams.SetLogStreamNamePrefix(log_stream_name.c_str());
+        auto const describe_outcome = aws_cloudwatch_client_->DescribeLogStreams(describe_log_streams);
+        if (describe_outcome.IsSuccess())
+        {
+            const auto& log_streams = describe_outcome.GetResult().GetLogStreams();
+            for (const auto& log_stream : log_streams)
+            {
+                if (log_stream.GetLogStreamName() == log_stream_name)
+                {
+                    existing_log_streams_.insert(log_stream_name);
+                    return;
+                }
+            }
+        }
         Aws::CloudWatchLogs::Model::CreateLogStreamRequest create_log_stream;
         create_log_stream.WithLogGroupName(log_group_name_.c_str()).WithLogStreamName(log_stream_name.c_str());
         auto const outcome = aws_cloudwatch_client_->CreateLogStream(create_log_stream);

@jcar87
Copy link
Contributor

jcar87 commented Jun 24, 2024

Please note that it 1.12.0 does not appear to be a release yet: https://github.com/ofiriluz/octo-logger-cpp/releases

so maybe the maintainers are updating the tag until it is.

furthermore, this is the second time we have a checksum issue: #24308

I can see that this will continue being a problem until 1.12.0 is actually released. Under what conditions was 1.12.0 considered a release? The presence of the tag?

@toge
Copy link
Contributor Author

toge commented Jun 24, 2024

@jcar87 @RubenRBS
Thank you for your deep dive.

I have updated this recipe as the 1.12.0 tag has been added.
I understand that this is an inconvenient situation for cci.
I will keep the use of 1.12.0 in my private environment.
Is there any way to make this clear to users that it will be difficult to use 1.12.0 on cci until an official release is made?

@jcar87
Copy link
Contributor

jcar87 commented Jun 26, 2024

Is there any way to make this clear to users that it will be difficult to use 1.12.0 on cci until an official release is made?

Thanks @toge ! We can probably invalidate all packages in the validate() method and point users to use an older version? Until a new recipe revision is published that has the actual release.
Or we can publish these as 1.12.0-pre1 etc - but point to the actual commit rather than the 1.12.0 tag - but the 1.12.0 will still remain published

Any ideas @RubenRBS ??

@perseoGI
Copy link
Contributor

I suggest removing support for this version as v2.0.0 #24743 has been released and it is getting merged soon.

@toge
Copy link
Contributor Author

toge commented Jul 29, 2024

Oh, I forget my old work.
Let's close it.

@toge toge closed this Jul 29, 2024
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.

5 participants