Skip to content

Conversation

@stepansergeevitch
Copy link
Collaborator

@stepansergeevitch stepansergeevitch commented Jun 22, 2023

Updated ResourceManager to use a new SQL api to handle resources

  • deprecated Settings usage in favor of passing parameter seprately
  • removed binding, region, provider and engine_revision models and services
  • fixed unit and integration tests

@stepansergeevitch stepansergeevitch self-assigned this Jun 22, 2023
@stepansergeevitch stepansergeevitch changed the title Fir 24158 new identity resource manager feat: Fir 24158 new identity resource manager Jun 22, 2023
Copy link
Contributor

@ptiurin ptiurin left a comment

Choose a reason for hiding this comment

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

Also, unit tests are kaput :'(

Comment on lines +101 to +107
def refresh(self) -> None:
"""Update attributes of the instance from Firebolt."""
field_name_overrides = self._get_field_overrides()
for name, value in self._service._get_dict(self.name).items():
setattr(self, field_name_overrides.get(name, name), value)

self.__post_init__()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not entirely clear why we're refreshing attributes here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We just fetch the engine state from db and update each class field with a new value.
Then we do post init for value post processing

Copy link
Contributor

@ptiurin ptiurin left a comment

Choose a reason for hiding this comment

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

Apart from that LGTM

@stepansergeevitch stepansergeevitch force-pushed the FIR-24158-new-identity-resource-manager branch from 204ea67 to ac9c7c7 Compare July 3, 2023 07:33
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 4, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

87.8% 87.8% Coverage
0.0% 0.0% Duplication

@stepansergeevitch stepansergeevitch merged commit 79baa7f into main Jul 4, 2023
@stepansergeevitch stepansergeevitch deleted the FIR-24158-new-identity-resource-manager branch July 4, 2023 12:52
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