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

Update/clean up #49

Merged
merged 12 commits into from
Jul 10, 2024
Merged

Update/clean up #49

merged 12 commits into from
Jul 10, 2024

Conversation

JettTech
Copy link
Contributor

@JettTech JettTech commented Jul 8, 2024

Note: This handles issue #48.

Context: This handles the issue @zo-el pointed out last Friday as well as few other bugs that were hard to find in the previous structure. It also cleans up the code base to support maintainability and knowledge sharing, with attn to supporting the ongoing sl rotation work.

Updates:

  • removes redundant types
  • groups code into related folders
  • changes admin call from list_running_app to list_apps(Some(AppStatusFilter::Enabled))
  • updates naming to be more explicit
  • cleans up helper fn and flow
  • fixes id ref bugs
  • fixes issue Not updating HHA when happ is disabled #48 (listed above)
  • removes the functionality to save the host's default happ pricing preferences to a file under {SL_PREFS_PATH}

@JettTech JettTech requested a review from zo-el as a code owner July 8, 2024 08:25
src/lib.rs Outdated Show resolved Hide resolved
src/types/hbs.rs Outdated Show resolved Hide resolved
Comment on lines 66 to 77
pub fn save(self) -> Result<Self> {
if let Ok(path) = env::var("SL_PREFS_PATH") {
trace!("Writing default servicelogger prefs to {}", &path);
// create or overwrite to a file
let file = File::create(&path)?;
serde_yaml::to_writer(file, &self).context(format!(
"Failed writing service logger preferences to file {}",
path
))?;
};
Ok(self)
}
Copy link
Member

Choose a reason for hiding this comment

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

Is this still in use? If this is a clean up this should be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is in use upon install of the hosted happs, in order to save the hard coded happ preferences to a file. To confirm, you're saying we no longer need to create and reference this preference file, correct?

Copy link
Member

Choose a reason for hiding this comment

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

After your implementation of getting the preferences from HHA for SL, I don't think we have been using this file.
The only place would be the hpos-api, and if that's the case that needs to be updated since that will no longer be accurate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. It does look like it's being used there. I'll make a PR to amend that out too.

src/types/hbs.rs Outdated Show resolved Hide resolved
use hpos_hc_connect::hha_types::HappAndHost;
use tracing::trace;

pub async fn get_host_preferences(core_app_client: &mut HHAAgent) -> Result<HappPreferences> {
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: All these zomeCalls could be added to the HHAAgent impl here: https://github.com/Holo-Host/hpos-service-crates/blob/main/crates/hpos_connect_hc/src/hha_agent.rs#L56-L89

Copy link
Contributor Author

@JettTech JettTech Jul 10, 2024

Choose a reason for hiding this comment

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

Done. Can you take a look and approve at the dependent pr: Holo-Host/hpos-service-crates#191

Co-authored-by: Joel Ulahanna <[email protected]>
@JettTech JettTech merged commit 713a176 into main Jul 10, 2024
2 checks passed
@JettTech JettTech deleted the update/clean-up branch July 10, 2024 18:03
zeeshan595 pushed a commit that referenced this pull request Jul 10, 2024
* jurisdictions and categories struct compat with HHA

* linting

* don't panic if none

* lint

* hpos_hc_connect: bump to latest

* update flake.nix

* holochain v0.4.0-dev.1 (#47)

* get_happ_preferences: update payload

* clippy

* edit names

* clean up & restructure

* move hbs types

* import download_file from hpos_hc_connect

* inline doc touch-ups

* clean up comments

* merge clean-up

* Update/clean up (#49)

* edit names

* clean up & restructure

* move hbs types

* import download_file from hpos_hc_connect

* inline doc touch-ups

* clean up comments

* Update src/types/hbs.rs

Co-authored-by: Joel Ulahanna <[email protected]>

* import core app types from hpos_hc_connect

* remove option from kyc

* edit comments

---------

Co-authored-by: Joel Ulahanna <[email protected]>

---------

Co-authored-by: Alastair Ong <[email protected]>
Co-authored-by: peeech <[email protected]>
Co-authored-by: Joel Ulahanna <[email protected]>
JettTech added a commit to Holo-Host/hpos-service-crates that referenced this pull request Jul 10, 2024
### Updates:
- Adds the zome calls in response to the suggestion from the
[holo-auto-installer pr
#49](Holo-Host/holo-auto-installer#49)
zeeshan595 added a commit that referenced this pull request Jul 11, 2024
* feat: added invoice due date from note

* feat: imports clean up

* feat: added mattermost notification for holo-auto-installer

* feat: removed un-used imports

* feat: removed non snake case error

* Merge/updated structure into invoice pr (#52)

* jurisdictions and categories struct compat with HHA

* linting

* don't panic if none

* lint

* hpos_hc_connect: bump to latest

* update flake.nix

* holochain v0.4.0-dev.1 (#47)

* get_happ_preferences: update payload

* clippy

* edit names

* clean up & restructure

* move hbs types

* import download_file from hpos_hc_connect

* inline doc touch-ups

* clean up comments

* merge clean-up

* Update/clean up (#49)

* edit names

* clean up & restructure

* move hbs types

* import download_file from hpos_hc_connect

* inline doc touch-ups

* clean up comments

* Update src/types/hbs.rs

Co-authored-by: Joel Ulahanna <[email protected]>

* import core app types from hpos_hc_connect

* remove option from kyc

* edit comments

---------

Co-authored-by: Joel Ulahanna <[email protected]>

---------

Co-authored-by: Alastair Ong <[email protected]>
Co-authored-by: peeech <[email protected]>
Co-authored-by: Joel Ulahanna <[email protected]>

* feat: cargo fmt

---------

Co-authored-by: Zeeshan Abid <[email protected]>
Co-authored-by: Lisa Jetton <[email protected]>
Co-authored-by: Alastair Ong <[email protected]>
Co-authored-by: peeech <[email protected]>
Co-authored-by: Joel Ulahanna <[email protected]>
JettTech added a commit that referenced this pull request Jul 15, 2024
…#54)

* feat: added invoice due date from note

* feat: imports clean up

* edit names

* clean up & restructure

* move hbs types

* import download_file from hpos_hc_connect

* inline doc touch-ups

* feat: added mattermost notification for holo-auto-installer

* feat: removed un-used imports

* clean up comments

* merge clean-up

* feat: removed non snake case error

* Merge/updated structure into invoice pr (#52)

* jurisdictions and categories struct compat with HHA

* linting

* don't panic if none

* lint

* hpos_hc_connect: bump to latest

* update flake.nix

* holochain v0.4.0-dev.1 (#47)

* get_happ_preferences: update payload

* clippy

* edit names

* clean up & restructure

* move hbs types

* import download_file from hpos_hc_connect

* inline doc touch-ups

* clean up comments

* merge clean-up

* Update/clean up (#49)

* edit names

* clean up & restructure

* move hbs types

* import download_file from hpos_hc_connect

* inline doc touch-ups

* clean up comments

* Update src/types/hbs.rs

Co-authored-by: Joel Ulahanna <[email protected]>

* import core app types from hpos_hc_connect

* remove option from kyc

* edit comments

---------

Co-authored-by: Joel Ulahanna <[email protected]>

---------

Co-authored-by: Alastair Ong <[email protected]>
Co-authored-by: peeech <[email protected]>
Co-authored-by: Joel Ulahanna <[email protected]>

* feat: cargo fmt

* clean-up suspended disable flow

* edit comment

---------

Co-authored-by: Zeeshan Abid <[email protected]>
Co-authored-by: Alastair Ong <[email protected]>
Co-authored-by: peeech <[email protected]>
Co-authored-by: Joel Ulahanna <[email protected]>
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