Skip to content

Conversation

@cahartma
Copy link

@cahartma cahartma commented Apr 8, 2025

Updated files to ensure cargo check runs successfully. Two vendor packages were updated to include files from the checksum (same package but different versions). One fix for tls file, adding the missing 'Connected' trait.

In addition to being able to successfully run cargo check --offline:

Now, we should be able to build dev and run all upstream unit tests:

git checkout v0.37.1-rh
make build-dev
make test

cc: @jcantrill @Clee2691 @vparfonov

}
}

#[derive(Clone)]

Choose a reason for hiding this comment

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

this changes also related to the cargo check?

Copy link
Author

@cahartma cahartma Apr 9, 2025

Choose a reason for hiding this comment

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

Yes, this fails for me every time. tls connected trait was not added correctly at implementation.


const fn should_encode_as_binary(&self) -> bool {
use vector_lib::codecs::encoding::Serializer::{
Avro, Csv, Gelf, Json, Logfmt, Native, NativeJson, Protobuf, RawMessage, Text,

Choose a reason for hiding this comment

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

Is this changes also related to the cargo check?

Copy link
Author

@cahartma cahartma Apr 9, 2025

Choose a reason for hiding this comment

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

Yes cargo check should fail when you run it without the changes, as syslog was not initially added to the implemented interfaces.

@vparfonov
Copy link

/lgtm

@jcantrill
Copy link
Member

/hold

for z-stream code freeze

@jcantrill
Copy link
Member

@cahartma do we need to update our CI to embrace these test changes?

# Jetbrains
.idea/

!vendor/
Copy link
Member

Choose a reason for hiding this comment

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

Why do we now need to vendor dependencies? Is it absolutely necessary? These don't look security critical but do we have a mechanism to ensure they get upgraded as either the vector version gets updated or we discover security flaws?

@jcantrill
Copy link
Member

/approve

@openshift-ci
Copy link

openshift-ci bot commented Apr 10, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cahartma, jcantrill

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cahartma
Copy link
Author

/unhold

@openshift-merge-bot openshift-merge-bot bot merged commit 8637524 into ViaQ:v0.37.1-rh Apr 18, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants