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

Port OS Environment varialbes to viper config #127

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jakeschuurmans
Copy link
Contributor

No description provided.

@jakeschuurmans jakeschuurmans marked this pull request as draft September 25, 2023 14:55
bucket:
access_key:
secret_key:
s3:

Choose a reason for hiding this comment

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

What's the difference between the s3 and asrr.s3 stanzas?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@DoctorVin s3 is the destination bucket. asrr s3 is the source bucket for AsRockrack firmware images. It's a hack because our firmware images for ASRR come from a ftp site that's not available publicly so at the time this was added we agreed to keep the fw images in that s3 bucket and sync from there instead of using the private ftp they provide.

err := v.ReadInConfig()

if err != nil {
logger.Error("Failed to find viper config file")

Choose a reason for hiding this comment

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

Should probably be a Fatal here.

@@ -67,7 +77,7 @@ func New(configFile string, logLevel int) (*Syncer, error) {
case common.VendorDell:
var dup vendors.Vendor

dup, err = dell.NewDUP(context.TODO(), firmwares, cfgSyncer, logger)
dup, err = dell.NewDUP(context.TODO(), firmwares, cfgSyncer, logger, v)

Choose a reason for hiding this comment

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

instead of passing the viper pointer, it would be nicer (IMO) to parse this into one (or several?) structs that were specific to the vendors?

@diogomatsubara
Copy link
Collaborator

@jakeschuurmans In branch https://github.com/metal-toolbox/firmware-syncer/tree/on-demand-sync I have the changes to align the env variable handling in the firmware-syncer with the metal-toolbox. Unfortunately it's a bit of a mess as I was cargo culting things over from flasher/cookie-flipper to get the on-demand syncing working and I pulled in the env var handling as well. Maybe it could help you and address some of Vince's comments.

@ofaurax
Copy link
Contributor

ofaurax commented Oct 3, 2024

Is this still a work in progress?

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.

4 participants