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.

Sign up for GitHub

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

Jump to bottom

Dev kmr #508

Merged
merged 6 commits into from
Jul 1, 2024
Merged

Dev kmr #508

merged 6 commits into from
Jul 1, 2024

Conversation

k-rister
Copy link
Contributor

No description provided.

@k-rister k-rister added enhancement New feature or request DO NOT MERGE labels Jun 28, 2024
@k-rister k-rister requested a review from a team June 28, 2024 14:34
@k-rister k-rister self-assigned this Jun 28, 2024
@k-rister k-rister mentioned this pull request Jun 28, 2024
Closed
@k-rister
Copy link
Contributor Author

Just a note, at the moment this code relies on this refresh-timestamps object to be present in rickshaw-settings.json:

...
    "workshop": {
        "force-builds": "false",
        "refresh-timestamps": {
            "token-file": "/root/quay-oauth.token",
            "api-url": "https://quay.io/api/v1/repository/crucible/client-server"
        }
    },
...

rafaelfolco
rafaelfolco previously approved these changes Jun 28, 2024
View reviewed changes
Copy link
Contributor

@rafaelfolco rafaelfolco left a comment

Choose a reason for hiding this comment

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

lgtm

rickshaw-run Outdated Show resolved Hide resolved
k-rister added 2 commits June 29, 2024 14:01
@k-rister
change the rickshaw-settings.json handling of the refresh-timestamps …
d6542f5 
…object

- switch from the object (and it's sub properties) being optional to
  being required with null values

- the intent is to make source code management of the file (via git)
  more straightforward since there will be easier to integrate
  upstream changes
@k-rister
swap tabs for spaces in rickshaw-settings.json
586fc94 
- this allows rickshaw-settings.json to be manipulated with tools such
  as jq and maintain consistency by using 4 space indendation

- consistency is key for seemless merging of upstream changes to the
  file
@k-rister
change refresh-timestamps to refresh-expiration and move from worksho…
e7ed2a7 
…p to quay in rickshaw-settings.json

- makes things more consistent and obvious
@k-rister
Copy link
Contributor Author

k-rister commented Jul 1, 2024

Just a note, at the moment this code relies on this refresh-timestamps object to be present in rickshaw-settings.json:

...
    "workshop": {
        "force-builds": "false",
        "refresh-timestamps": {
            "token-file": "/root/quay-oauth.token",
            "api-url": "https://quay.io/api/v1/repository/crucible/client-server"
        }
    },
...

This has been refactored into this:

...
    "quay": {
        "image-expiration": "2w",
        "refresh-expiration": {
            "token-file": "/root/crucible-internal/crucible-production-quay-oauth.token",
            "api-url": "https://quay.io/api/v1/repository/crucible/client-server"
        }
    }
...

It just makes more sense for this to be put in the quay object and hopefully it's more logical to call it refresh-expiration instead of refresh-timestamps.

rafaelfolco
rafaelfolco approved these changes Jul 1, 2024
View reviewed changes
Copy link
Contributor

@rafaelfolco rafaelfolco left a comment

Choose a reason for hiding this comment

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

lgtm

rickshaw-settings.json Show resolved Hide resolved
rickshaw-settings.json Outdated Show resolved Hide resolved
@k-rister k-rister merged commit fd6730c into master Jul 1, 2024
186 checks passed
@k-rister k-rister deleted the dev-kmr branch July 1, 2024 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rafaelfolco rafaelfolco rafaelfolco approved these changes

Assignees

@k-rister k-rister

Labels
enhancement New feature or request
Projects
Crucible Tracking
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@k-rister @rafaelfolco