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

DataSync: Added error code, method, property and Enum #160

Closed
wants to merge 3 commits into from

Conversation

harsh-agarwal1
Copy link

  • Introduced the "SiblingNotAvailable" error code specific to
    DataSync interfaces.
  • Introduced the "FullSync" method interface to initiate a full
    sync of the configured files and directory to the sibling BMC.
  • Introduced the "SyncStatus" property and Enum to define the
    Synchronization status of the BMC.

@jenkins-openbmc-ibm
Copy link

Can one of the admins verify this patch?

Start a full Sync of all the configured files and directory, to
the sibling BMC.
errors:
- xyz.openbmc_project.DataSync.BMCData.Error.SiblingNotAvailable
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we return an error if the FullSync is in progress, or would you prefer to handle this based on the error handling logic?

methods:
- name: FullSync
description: >
Start a full Sync of all the configured files and directory, to
Copy link
Contributor

Choose a reason for hiding this comment

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

a full Sync of all the configured files and directory

Thanks for capturing this but two point need to consider.

  1. It won't sync all configured files and directories; the sync will be based on the role, correct? Please make sure to capture that information as well.

  2. We have one more use case: some firmware subsystems or their components need to sync their data based on specific requirements rather than immediately or periodically. Since the OpenBMC DBus YAML framework does not support default arguments (i.e., FullSync with an argument to specify the required files—if the argument is not provided, it defaults to syncing all files), we cannot use this API for such cases. Therefore, we will need a separate method for this, but we can consider it once we have finalized the end-to-end flow between firmware subsystems (HB/PHYP, etc.).

and I believe we should include details on how we plan to track the status of the full sync.

type: enum[self.SyncStatus]
default: Unknown
description: >
This property represents the current status of the synchronization
Copy link
Contributor

Choose a reason for hiding this comment

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

current status of the synchronization

I think you meant the full sync status.

The current sync status won’t be very useful without specifying details about the specific files or directories. I believe these factors should also be considered from a statistics perspective. Let's make sure to clarify this in the documentation for this status property.

yaml/xyz/openbmc_project/DataSync/BMCData.interface.yaml Outdated Show resolved Hide resolved
- Introduced the SiblingNotAvailable error code specific to
DataSync interfaces.
- This error should be triggered when a request involves a
sibling BMC that is unavailable.

Change-Id: Ica32191154b7eb6b0a806fe45249f733352eae7f
Signed-off-by: Harsh Agarwal <[email protected]>
- This interface will be used to initiate a full sync of the
configured files and directory to the sibling BMC based on the role.

The added method will throw below exceptions.
- xyz.openbmc_project.DataSync.BMCData.Error.SiblingNotAvailable
  - If the sibling is not available for sync.

Change-Id: I1ac2375e807ca04a331b784d56a923bf894619a6
Signed-off-by: Harsh Agarwal <[email protected]>
- This property is used to define the full Synchronization status of
the BMC.
- This property can take values: "Unknown" and "FullSyncCompleted".

Change-Id: Ifedc8a6767db4d199b7dd00de77dd4e566346623
Signed-off-by: Harsh Agarwal <[email protected]>
@RameshIyyar
Copy link
Contributor

As discussed, I am waiting for upstream approval of the changes, as the DBus interface directory has been updated based on the community's suggestion.

This PR needs to incorporate those changes, and I noticed that a few comments have not been addressed yet.

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.

3 participants