Skip to content

Fix /Library/LaunchDaemons/limit.maxfiles.plist on MacOS #220

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
Merged
merged 1 commit into from
Jun 19, 2018
Merged

Fix /Library/LaunchDaemons/limit.maxfiles.plist on MacOS #220

merged 1 commit into from
Jun 19, 2018

Conversation

noelmcloughlin
Copy link
Contributor

Fix #213

@noelmcloughlin
Copy link
Contributor Author 

          ID: postgres_maxfiles_limits_conf
    Function: file.managed
        Name: /Library/LaunchDaemons/limit.maxfiles.plist
      Result: True
     Comment: File /Library/LaunchDaemons/limit.maxfiles.plist updated
     Started: 20:18:53.010893
    Duration: 37.051 ms
     Changes:   
              ----------
              diff:
                  --- 
                  +++ 
                  @@ -1,3 +1,21 @@
                  -      maxfiles
                  -      string {{ soft }}
                  -      string {{ hard }}
                  +<?xml version="1.0" encoding="UTF-8"?>  
                  +<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN"  
                  +        "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
                  +<plist version="1.0">  
                  +  <dict>
                  +    <key>Label</key>
                  +    <string>limit.maxfiles</string>
                  +    <key>ProgramArguments</key>
                  +    <array>
                  +      <string>/bin/launchctl</string>
                  +      <string>limit</string>
                  +      <string>maxfiles</string>
                  +      <string>64000</string>
                  +      <string>64000</string>
                  +    </array>
                  +    <key>RunAtLoad</key>
                  +    <true/>
                  +    <key>ServiceIPC</key>
                  +    <false/>
                  +  </dict>
                  +</plist>

vutny
vutny suggested changes Jun 16, 2018
View reviewed changes
Copy link
Contributor

@vutny vutny left a comment

Choose a reason for hiding this comment

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

Hi @noelmcloughlin .

Although your changes are syntactically correct, I don't see any of these limits defined by default for OS X.
They do exist in the pillar.example file, but I think for the template really to be working "out-of-the-box", we need to add the limit values to the osfamilymap.yaml file as well.

@noelmcloughlin
Copy link
Contributor Author

Hi @vutny I think 64K is a good assumed default for Darwin OSX. Yes for completeness I'll add default values.

@aboe76
Copy link
Contributor

aboe76 commented Jun 16, 2018

@vutny can you take a second look before merge?

vutny
vutny reviewed Jun 17, 2018
View reviewed changes
postgres/dev.sls Outdated
soft_limit: {{ postgres.limits.soft or postgres.limits.hard }}
hard_limit: {{ postgres.limits.hard or postgres.limits.soft }}
soft_limit: {{ postgres.limits.soft | default(postgres.limits.hard, true) }}
hard_limit: {{ postgres.limits.hard | default(postgres.limits.soft, true) }}
Copy link
Contributor

Choose a reason for hiding this comment

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

@noelmcloughlin Since we have added the default values to the OS family map, they would be merged to the postgres dictionary anyway if nothing set in the Pillar. So the default filter becomes unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks for the review @vutny

@aboe76 aboe76 merged commit a827825 into saltstack-formulas:master Jun 19, 2018
@aboe76
Copy link
Contributor

aboe76 commented Jun 19, 2018

@vutny and @noelmcloughlin thanks for this

@noelmcloughlin noelmcloughlin deleted the macos_fix branch June 19, 2018 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@vutny vutny vutny requested changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@noelmcloughlin @aboe76 @vutny