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

Implement selective update for HostAgentBean and add unit tests #1770

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tylerwowen
Copy link
Contributor

This pull request includes several changes to the deploy-service module, focusing on updating the HostAgentBean class and related components to support partial updates. The most important changes include adding a new method to generate a set clause for changed fields, modifying the HostAgentDAO and DBHostAgentDAOImpl classes to use this new method, and updating the PingHandler class to utilize these changes. Additionally, a new test class for HostAgentBean has been added.

Enhancements to HostAgentBean:

Updates to DAO classes:

Changes to PingHandler:

New test class:

These changes improve the efficiency of updates to HostAgentBean by ensuring that only the modified fields are updated in the database.

@tylerwowen tylerwowen requested a review from a team as a code owner December 20, 2024 18:48
@github-actions github-actions bot added the deploy-service Includes changes to deploy-service label Dec 20, 2024
Copy link
Contributor

@rwxzhu rwxzhu left a comment

Choose a reason for hiding this comment

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

lgtm

Comment on lines +66 to +71
public void updateChanged(String hostId, HostAgentBean oldBean, HostAgentBean newBean)
throws SQLException {
SetClause setClause =
oldBean == null ? newBean.genSetClause() : newBean.genChangedSetClause(oldBean);
String clause = String.format(UPDATE_HOST_BY_ID, setClause.getClause());
setClause.addValue(id);
setClause.addValue(hostId);
Copy link
Contributor

@Tweedeldee Tweedeldee Dec 20, 2024

Choose a reason for hiding this comment

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

passing the hostId as a parameter while it's already contained in both beans is a bit weird, but I guess it can serve as outlining how special that field is ?
I can't tell if I like it or hate it, so I'm ok with status quo.

Copy link
Contributor

@osoriano osoriano left a comment

Choose a reason for hiding this comment

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

Looks like a good optimization

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy-service Includes changes to deploy-service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants