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

Enhance builder info #1689

Merged
merged 1 commit into from
Jan 12, 2022
Merged

Enhance builder info #1689

merged 1 commit into from
Jan 12, 2022

Conversation

sajjaphani
Copy link
Contributor

@sajjaphani sajjaphani commented Jan 5, 2022

This PR handles to add/update additional information in the following API:

Existing package metadata APIs have been incorporated to return the size of the hart file. Below is an example:
https://bldr.habitat.sh/v1/depot/channels/core/stable/pkgs/7zip/latest?target=x86_64-linux

The APIs returns the below additional property JSON response:
{ ...., size: 1569, .... }

Change the channels API to include the promotion timestamp: /depot/pkgs/{origin}/{pkg}/{version}/{release}/channels
The API returns the below JSON response:
[ { name: 'unstable', created_at: '2022-01-05T11:09:34.245636', promoted_at: '2022-01-05T11:09:34.245636' }, { name: 'foo', created_at: '2022-01-05T11:09:37.385983', promoted_at: '2022-01-05T11:09:37.385983' } ]

Note: UI changes will be handled in a separate PR.

@sajjaphani sajjaphani requested a review from mwrock as a code owner January 5, 2022 13:29
@mwrock
Copy link
Contributor

mwrock commented Jan 5, 2022

Any reason why you would not add the size to do_get_package adding:

pkg_json["size"] = json!(req_state(&req).packages.size_of(&ident, target).await);

Note that the above won'r compile because size_of returns a Result but I'm just trying to convey the gist of how it might look.

This would eliminate the redundant code in the extra API and also cache the size along with the rest of the metadata in memcache.

@sajjaphani
Copy link
Contributor Author

Any reason why you would not add the size to do_get_package adding:

pkg_json["size"] = json!(req_state(&req).packages.size_of(&ident, target).await);

Note that the above won'r compile because size_of returns a Result but I'm just trying to convey the gist of how it might look.

This would eliminate the redundant code in the extra API and also cache the size along with the rest of the metadata in memcache.

We could do that, but it requires us to change the signature of do_get_package and all the other functions that depend on this.

We also need to check and invalidate the cache entry for package metadata, where the size attribute is missing.

@mwrock
Copy link
Contributor

mwrock commented Jan 6, 2022

How would you need to change the signature? It seems like you just need ident and target which do_get_package has. At least by the point in the function where it is ready to fetch the metadata it has a target and fully qualified ident.

Regarding cache invalidation, we could restart the memcache services after the release to clear out the cache.

@sajjaphani
Copy link
Contributor Author

We need to change function do_get_package to async, and then handle calling this function async.

Restarting will eventually erases everything including sessions. Restarting Memcached service should also happen on on-prem end. Can we automate this?

Copy link
Contributor

@mwrock mwrock left a comment

Choose a reason for hiding this comment

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

This looks good! I just think we need to remove the extra Size counter.

@mwrock
Copy link
Contributor

mwrock commented Jan 11, 2022

Per our offline conversation, it does look like the on-prem upgrade process involves a supervisor restart: https://github.com/habitat-sh/on-prem-builder/blob/main/on-prem-docs/upgrading.md

@sajjaphani
Copy link
Contributor Author

sajjaphani commented Jan 11, 2022

This looks good! I just think we need to remove the extra Size counter.

SizeRequest is update on call to size_of in s3.rs (AWS s3 API call), I think it may be good idea to have this metric.

@mwrock
Copy link
Contributor

mwrock commented Jan 11, 2022

I was concerned the size counter would mirror the GetPackage counter but it looks like GetPackage will get called on every call to do_get_package but the size counter would only be called if the json was not cached. Sorry for the confusion. Feel free to add it back. Can you also squash the commits?

Add published time to the channels API

Signed-off-by: Phani Sajja <[email protected]>
@sajjaphani sajjaphani force-pushed the psajja/enhance-bldr-info branch from 34b21b9 to b2f8fa7 Compare January 11, 2022 17:20
@sajjaphani
Copy link
Contributor Author

Squashed all the commits and should be good now! @mwrock Can you take a quick look at this?

@sajjaphani sajjaphani merged commit 1d97217 into main Jan 12, 2022
@sajjaphani sajjaphani deleted the psajja/enhance-bldr-info branch January 12, 2022 10:01
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.

2 participants