config should be validated (like a schema check)

[altendky]

User reported some confusing issues with paths. Turns out they had tmp: /my/path instead of tmp: [/my/path] (or a multiline yaml list). The config file should be checked against a schema before we go using it. The following code expects a list but is just fine taking a string '/my/path' and turning it into a bunch of temporary directories like ['/', 'm', 'y', '/', 'p', 'a', 't', 'h']. This bug (we guess) also resulted in temporary files getting dumped in / which strikes me as extra bad.

        tmp: /root/venvChia/plots/temp1

plotman/manager.py

Lines 79 to 80 in 31ccb8f

	tmp_to_all_phases = [ (d, job.job_phases_for_tmpdir(d, jobs))
	for d in dir_cfg['tmp'] ]

complete config file

# Where to plot and log.
directories:
        # One directory in which to store all plot job logs (the STDOUT/
        # STDERR of all plot jobs).  In order to monitor progress, plotman
        # reads these logs on a regular basis, so using a fast drive is
        # recommended.
        log: /root/venvChia/plotman/logs
 
        # One or more directories to use as tmp dirs for plotting.  The
        # scheduler will use all of them and distribute jobs among them.
        # It assumes that IO is independent for each one (i.e., that each
        # one is on a different physical device).
        #
        # If multiple directories share a common prefix, reports will
        # abbreviate and show just the uniquely identifying suffix.
        tmp: /root/venvChia/plots/temp1
 
 
 
        # Optional: tmp2 directory.  If specified, will be passed to
        # chia plots create as -2.  Only one tmp2 directory is supported.
        # tmp2:
 
        # One or more directories; the scheduler will use all of them.
        # These again are presumed to be on independent physical devices,
        # so writes (plot jobs) and reads (archivals) can be scheduled
        # to minimize IO contention.
        dst: /root/venvChia/plots/plot4
 
        # Archival configuration.  Optional; if you do not wish to run the
        # archiving operation, comment this section out.
        #
        # Currently archival depends on an rsync daemon running on the remote
        # host, and that the module is configured to match the local path.
        # See code for details.
        # archive:
        #        rsyncd_module: plots
        #        rsyncd_path: /plots
        #        rsyncd_bwlimit: 80000  # Bandwidth limit in KB/s
        #        rsyncd_host: myfarmer
        #        rsyncd_user: chia
 
 
# Plotting scheduling parameters
scheduling:
        # Don't run a job on a particular temp dir until all existing jobs
        # have progresed at least this far.  Phase major corresponds to the
        # plot phase, phase minor corresponds to the table or table pair
        # in sequence.
        tmpdir_stagger_phase_major: 2
        tmpdir_stagger_phase_minor: 1
 
        # Don't run more than this many jobs at a time on a single temp dir.
        tmpdir_max_jobs: 1
 
        # Don't run any jobs (across all temp dirs) more often than this.
        global_stagger_m: 30
 
        # How often the daemon wakes to consider starting a new plot job
        polling_time_s: 1000
 
 
# Plotting parameters.  These are pass-through parameters to chia plots create.
# See documentation at
# https://github.com/Chia-Network/chia-blockchain/wiki/CLI-Commands-Reference#create
plotting:
        k: 32
        e: True              # Use -e plotting option
        n_threads: 4         # Threads per job
        n_buckets: 128       # Number of buckets to split data into
        job_buffer: 3300     # Per job memory

[jkbecker]

StrictYAML can be used for that. Heads up: it's kind of a pain to use compared to regular simple YAML. But does exactly what's on the tin (typechecking & structure checking for the entire config file, no code execution from YAML, etc)

[altendky]

Would there be interest in having class instances come out rather than just dicts?

My go-to would be https://pypi.org/project/desert/ (built on Marshmallow and attrs). I forget the exact incantation but it should be able to process the existing deserialized YAML. The result is instances of the classes we write. We get to use attrs which... is different, but well loved. It does encourage a somewhat different class design, but I think that ends up being a good thing. For example, it encourages lightweight class construction which would certainly make things easier to test (https://github.com/ericaltendorf/plotman/pull/93/files#diff-b47b09c36a0de6fe9ad4fe9b174d831bc1719863281411508fbf959af8ebce1bR11-R19). Reading the logfile twice with potential for multiple 1 second sleeps is a bit much for Job.__init__(). :]

[jkbecker]

Didn't know about desert, that actually looks really nice. StrictYAML is just for schema enforcement but it doesn't help you instantiate classes from the data.

[ericaltendorf]

Sounds fine to me.

…

On Wed, Apr 7, 2021 at 8:04 AM Johannes Becker ***@***.***> wrote: Didn't know about desert, that actually looks really nice. StrictYAML is just for schema enforcement but it doesn't help you instantiate classes from the data. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#77 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ARPZIFXRR7WDQEJ4OXUYJJTTHRX7FANCNFSM42KI2ZGA> .

[chelseadole]

Hey there! I'm new to this project, and have been using plotman to set up my second Chia plotting box. I'm interested in contributing to the project, and have used python yaml libraries before (though desert is new to me). Is this a good issue for a new contributor to take on? @ericaltendorf @altendky :)

[altendky]

@chelseadole, if you are interested in learning desert (and attrs if that's new), then seems like a good start to me. :] I think it would make sense to build off of #98, or wait for it to be merged. It centralizes the configuration loading and gives a place for defining the configuration classes.

It looks like data in the example at https://github.com/python-desert/desert#usage would correspond to what we have now. So, we can stick with the (now safe) YAML loading we have, then feed the resulting dicts/lists/strings/etc through the to-be-written attrs/desert classes.

Thanks for the help! Feel free to ask any questions here or on keybase.

[chelseadole]

I'll give it a go, basing off #98. And desert looks nice! At first glance, it reminds me a bit of pydantic -- which I like a lot 👍

[jkbecker]

Cool! Just leaving a pointer here, I just rambled a bit about config file handling and validation on the CLI level over in #84.

I'm happy to take on that side of the equation, but I'm pretty busy at the moment - if you end up buildin a solution that covers both the actual YAML validation as well as how config files are handled generally, knock yourself out 😃. I'll check back here before spending any time on it, when I actually end up having time to spend.