Skip to content

Conversation

al1img
Copy link
Collaborator

@al1img al1img commented Sep 24, 2025

No description provided.

Copy link
Contributor

@mykola-kobets-epam mykola-kobets-epam left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Mykola Kobets <[email protected]>

Copy link
Contributor

Choose a reason for hiding this comment

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

common: tools: crate core alert structs
=> create

ResourceInfoStaticArray mResources;
RuntimeInfoStaticArray mRuntimes;
ResourceInfoArray mResources;
RuntimeInfoArray mRuntimes;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove Array suffix as well, eg:
ResourceInfoArary => Resources,
RuntimeInfoArray => Runtimes

Copy link

@MykolaSuperman MykolaSuperman left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Mykola Solianko <[email protected]>

Copy link
Member

@mlohvynenko mlohvynenko left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Mykhailo Lohvynenko <[email protected]>

@al1img al1img force-pushed the update_cloud_protocol branch from 95ed5c5 to db9424b Compare September 24, 2025 12:53
@al1img al1img force-pushed the update_cloud_protocol branch from db9424b to f8126ae Compare September 29, 2025 09:06
Move common part to types and use local env vars struct in core lib.

Signed-off-by: Oleksandr Grytsov <[email protected]>
Move common part to types and use local log struct in core lib.

Signed-off-by: Oleksandr Grytsov <[email protected]>
@al1img al1img force-pushed the update_cloud_protocol branch from 2e4e3dd to 11107b4 Compare September 29, 2025 16:36
struct SystemQuotaAlert : AlertItem {
Identity mNode;
StaticString<cAlertParameterLen> mParameter;
size_t mValue;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
size_t mValue;
size_t mValue{};

*/
struct InstanceQuotaAlert : AlertItem, InstanceIdent {
StaticString<cAlertParameterLen> mParameter;
size_t mValue;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
size_t mValue;
size_t mValue{};

*/
struct DownloadAlert : AlertItem {
StaticString<cIDLen> mImageID;
size_t mDownloadedBytes;
Copy link
Member

@mlohvynenko mlohvynenko Sep 29, 2025

Choose a reason for hiding this comment

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

Suggested change
size_t mDownloadedBytes;
size_t mDownloadedBytes{};
size_t mTotalBytes{};

Optional<AlertRules> mAlertRules;
Optional<ResourceRatios> mResourceRatios;
StaticArray<StaticString<cLabelNameLen>, cMaxNumNodeLabels> mLabels;
uint64_t mPriority {0};
Copy link
Member

Choose a reason for hiding this comment

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

Quite an optional comment, should discuss with team:

Suggested change
uint64_t mPriority {0};
uint64_t mPriority {};

Currently we have the next POD initialization approaches:

  1. uint64_t mPriority {};
  2. uint64_t mPriority {0};
  3. uint64_t mPriority = 0;
  4. uint64_t mPriority ; // trash - initialized

It seems to me the 1st one approach is the most used

struct InstanceInfo {
Identity mIdentity;
Identity mSubject;
uint64_t mPriority {0};
Copy link
Member

Choose a reason for hiding this comment

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

ditto (the same very optional comment)

struct PushLog {
StaticString<cLogIDLen> mLogID;
Identity mNode;
uint64_t mPartsCount;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uint64_t mPartsCount;
uint64_t mPartsCount{};

StaticString<cLogIDLen> mLogID;
Identity mNode;
uint64_t mPartsCount;
uint64_t mPart;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uint64_t mPart;
uint64_t mPart{};

*/
struct NodeStateInfo {
Time mTimestamp {};
bool mProvisioned = false;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
bool mProvisioned = false;
bool mProvisioned {};

ResourceInfoArray mResources;
RuntimeInfoArray mRuntimes;
NodeAttributeArray mAttrs;
bool mProvisioned = false;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
bool mProvisioned = false;
bool mProvisioned {};

struct PushLog {
StaticString<cLogIDLen> mLogID;
StaticString<cNodeIDLen> mNodeID;
uint64_t mPartsCount;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uint64_t mPartsCount;
uint64_t mPartsCount{};

StaticString<cLogIDLen> mLogID;
StaticString<cNodeIDLen> mNodeID;
uint64_t mPartsCount;
uint64_t mPart;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uint64_t mPart;
uint64_t mPart{};

@al1img al1img force-pushed the update_cloud_protocol branch from 11107b4 to d08416f Compare September 30, 2025 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants