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

aqd.conf.noms: update inappropriate defaults #79

Closed
wants to merge 1 commit into from

Conversation

jouvin
Copy link
Contributor

@jouvin jouvin commented May 18, 2018

  • Fix wrong ant_home: define as /usr/share/ant instead of the empty string
  • Enable management of git-daemon by the broker
  • Set panc output format to json
  • Add Postgres database name

Fixes #78
Fixes #82

@jouvin jouvin requested a review from urbonegi May 18, 2018 20:29
@jrha jrha changed the title aqd.conf.nomss: fix wrong ant_home aqd.conf.noms: fix wrong ant_home May 21, 2018
@jouvin jouvin changed the title aqd.conf.noms: fix wrong ant_home aqd.conf.noms: update inappriate defaults May 21, 2018
@jouvin jouvin changed the title aqd.conf.noms: update inappriate defaults aqd.conf.noms: update inapropriate defaults May 21, 2018
@jouvin jouvin changed the title aqd.conf.noms: update inapropriate defaults aqd.conf.noms: update inappropriate defaults May 21, 2018
dbname = aquilon

[database_sqlite]
dbfile = /pdisk/aquilon/db/aquilon.db

Choose a reason for hiding this comment

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

maybe it should be relative path to quattordir as in default conf?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@urbonegi sure, you are perfectly right! A too quick copy/paste.

dbname = aquilon

[database_sqlite]
dbfile = %(basedir)s/aquilon/db/aquilon.db

Choose a reason for hiding this comment

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

Just double checking. So if you would just skip this section the dbfile would be this:
dbfile = %(quattordir)s/aquilondb/aquilon.db
Should this be different really :) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@urbonegi I apologize for this serie of mistakes. You are perfectly right: it should be quattordir. I'd suggest to keep it here despite this is the default to better expose it, as this is one option that a site may want to modify. But if you prefer the other way around, I will not insist!

Choose a reason for hiding this comment

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

As this is a file for community rather than MS, i do not mind. Just not sure if having a ‘db’ subdirectory in ‘aquilon’ directory was intentional or a typo that turned ‘aquilondb’ into to ‘aquilon/db’ by accident?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your eyes. Clearly another mistake I didn't spot!

@jouvin jouvin force-pushed the aqd_conf branch 2 times, most recently from d82428a to 8ff8f3d Compare May 23, 2018 06:23
@urbonegi
Copy link

Great! @xaf just noticed that all the commits missing the change-id.. Please add it cause if we will be adding it ourselves this will change commitid of the commits. Instructions here:https://www.quattor.org/aquilon/00-install.html#contributing

@jouvin
Copy link
Contributor Author

jouvin commented May 23, 2018

Sure, I'll add it. Also not that I'm changing the include_pan back to true as it doesn't fix the real problem (#86).

@jouvin
Copy link
Contributor Author

jouvin commented May 23, 2018

Should be ok now! (i'll fix the missing Change-Id in my other pending PRs too).

@Shaeli
Copy link

Shaeli commented May 24, 2018

@jouvin could you update the commit message to remove the extra 's' please ?

- Fix wrong ant_home: define as /usr/share/ant instead of the empty string
- Enable management of git-daemon by the broker
- Set panc output format to json
- Add Postgres database name

Fixes quattor#78

Change-Id: Ibf218a69dd149cfe63d6af20c406000b524ed028
@jouvin
Copy link
Contributor Author

jouvin commented May 24, 2018

@Shaeli Sorry for the typo. Should be ok now.

@urbonegi
Copy link

not releasing this one now, as it depends on PR #87 and #89

@jouvin
Copy link
Contributor Author

jouvin commented May 30, 2018

@urbonegi I don't understand your last comment. Except if I did a mistake, this PR is completely independent of #87 (no change related to this issue). If you merge it first, I can rebase #87 on it afterwards. But it can also be the other way around if you prefer...

@urbonegi
Copy link

urbonegi commented Jul 3, 2018

merge: 3eef77c

@urbonegi urbonegi closed this Jul 3, 2018
ned21 pushed a commit to ned21/aquilon that referenced this pull request Mar 4, 2020
…merge/master/by_topic/barnesf_disk_parameters to master

* commit '969ca7f9bd25839fab07e75ad4bf0b638f6c4d48':
  Add new fields to Disk object
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants