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

Fixes #37481 - Update cached value for manifest expiration date during import #10999

Merged
merged 1 commit into from
May 22, 2024

Conversation

jeremylenz
Copy link
Member

@jeremylenz jeremylenz commented May 16, 2024

Our stored manifest expiration date relies on a Rails cached value which is held for 1 minute:

def manifest_expiration_date(cached: true)
          Rails.cache.fetch("#{self.label}_manifest_expiration_date", expires_in: 1.minute, force: !cached) do

This means that after a manifest import / refresh, the manifest expiration date may be inaccurate until the cache expiration time (1 minute). Because of this you must refresh the browser page to see the new manifest expiration date.

What are the changes introduced in this pull request?

  1. During manifest import (and as a consequence, during manifest refresh), I found that calling manifest_expiration_date(cached: false) is enough to prevent stale data from making it to the user's screen.
  2. If the manifest expiration date is not populated for some reason, don't display anything.

Considerations taken when implementing this change?

What are the testing steps for this pull request?

Delete and reimport a manifest. Visit the Manage Manifest modal.

Before:
You'll see a manifest expiration date of the Unix epoch (31 Dec 1969 or 01 Jan 1970)

After:
You'll see the correct manifest expiration date

Copy link
Member

@jturel jturel left a comment

Choose a reason for hiding this comment

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

Nice work @jeremylenz - thanks for the PR! :)

@chris1984
Copy link
Member

Nice work @jeremylenz - thanks for the PR! :)

We miss you man ❤️

@jeremylenz
Copy link
Member Author

Updated to better handle importing of already-expired manifests.

@jturel
Copy link
Member

jturel commented May 17, 2024

Nice work @jeremylenz - thanks for the PR! :)

We miss you man ❤️

Likewise!

@chris1984 chris1984 self-assigned this May 21, 2024
Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

Works great, thanks @jeremylenz

@jeremylenz jeremylenz merged commit 2933094 into Katello:master May 22, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants