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

Config spec deprecations #3404

Closed
hjoliver opened this issue Oct 9, 2019 · 12 comments
Closed

Config spec deprecations #3404

hjoliver opened this issue Oct 9, 2019 · 12 comments
Labels
duplicate This is a duplicate of something else

Comments

@hjoliver
Copy link
Member

hjoliver commented Oct 9, 2019

Taken from: #3348 (review)

Copying to 1 item per comment below, because most items have too much discussion for a simple list in the description here.

@hjoliver
Copy link
Member Author

hjoliver commented Oct 9, 2019

[general] needs a better name. The settings therein are not particularly "general" in most cases, they are merely a miscellaneous collection of settings that don't belong elsewhere.

I suggest [misc].

@hjoliver
Copy link
Member Author

hjoliver commented Oct 9, 2019

 'reference test': {
            'expected task failures': [VDR.V_STRING_LIST],
        },

This is now the only item left in the reference test section. I suggest we get rid of the section, and amend the above events section:

  • change abort if any task fails to abort on unexpected task failures
  • add expected task failures to replace the reference test item

(it doesn't really matter that this feature is currently only used in reference tests ... I think it could potentially be useful elsewhere).

(I think @oliver-sanders agreed with this on the origin discussion).

@hjoliver
Copy link
Member Author

hjoliver commented Oct 9, 2019

    'simulation': {
        'disable suite event handlers': [VDR.V_BOOLEAN, True],
    },
    'environment': {

@hjoliver
hjoliver 18 hours ago Member

This does not really belong under general (or misc). It is specifically the environment of the server program. Which used to be one way of handing static information to event handlers. Not sure it is ever needed now (do you know @oliver-sanders ?). If we keep it, can we change it to top level server environment?
@wxtim
wxtim 13 hours ago Author Member

Assuming that we don't want to delete it, do you mean suite host by server, or something else?
@oliver-sanders
oliver-sanders 11 hours ago Member

I think it makes sense to be able to configure the environment of the server program for xtriggers and, especially looking forward to Python API.

@hjoliver
Copy link
Member Author

hjoliver commented Oct 9, 2019

The [general][events] section could be moved to top level and renamed server events - to clearly distinguish from task events.

@oliver-sanders
oliver-sanders 11 hours ago Member

Seems like we have multiple "server" settings, perhaps group them in a section...
@hjoliver

@hjoliver
Copy link
Member Author

hjoliver commented Oct 9, 2019

[general][authorization] should be moved to a top level item.

@hjoliver
Copy link
Member Author

hjoliver commented Oct 9, 2019

Is suite definition directory still needed on job hosts?

I think(?) it is only used to set the value of CYLC_SUITE_DEF_PATH in job file environment, which in turn is used to put the suite bin directory in PATH ... if so we should change to a more sensible way of doing that (e.g. CYLC_SUITE_BIN_DIR or whatever ... actually these days everything gets installed to the run dir, should never need the "definition dir" on job hosts.

@hjoliver
Copy link
Member Author

hjoliver commented Oct 9, 2019

There was a comment somewhere else recently on the confusing nature of run vs work dir configuration. (One is a sub-dir of the other by default, but they are configured separately at the top run-dir level). Can we make this less confusing?

@hjoliver
Copy link
Member Author

hjoliver commented Oct 9, 2019

            'exclude at start-up': [VDR.V_STRING_LIST],
            'include at start-up': [VDR.V_STRING_LIST],

Beta Send feedback
Comment for lines +228 – +229
@hjoliver
hjoliver 18 hours ago Member

Does anyone ever use these two? They are (I think?) an ancient bodge made obsolete by R1 graphs.
@hjoliver
hjoliver 18 hours ago Member

(i.e. can we drop them?)
@wxtim
wxtim 13 hours ago Author Member

Seems reasonable, but I'd like thoughts from @oliver-sanders and @dpmatthews
@oliver-sanders
oliver-sanders 9 hours ago Member

These aren't deprecated so it would be pretty cheeky to drop them, however, none of the 600 suites currently running use these either...
@hjoliver
hjoliver now Member

(again, by drop I meant deprecate grimacing ... we never just literally remove items)

@hjoliver
Copy link
Member Author

hjoliver commented Oct 9, 2019

I know runtime dates back forever, but it is ambiguous (server program runtime, or task jobs?). Can we change it to something like task runtime, or job configuration, or jobs?
@hjoliver
hjoliver 18 hours ago Member

(probably not jobs)
@wxtim
wxtim 13 hours ago Author Member

I like this idea, but again this is one for further comment from @oliver-sanders and @dpmatthews. Other possibilities we might consider (just throwing ideas about):

tasks
tasks runtimes
task configurations

@oliver-sanders
oliver-sanders 9 hours ago Member

Is it really worth the user pain to change this?
@wxtim
wxtim 9 hours ago Author Member

Only if we can agree a definitively better term?
@hjoliver
hjoliver 36 seconds ago Member

Is it really worth the user pain to change this?

Yes! Changing a single item name by deprecation (perhaps supported longer-term because it's a big one) is not much pain. In light of recent discussions we need to be getting rid of anything that looks non-standard (where there is a "standard" terminology) or quirky or confusing. I took a step back from what I'm used to seeing every day, and thought "what in hell does 'runtime' mean" - there is nothing in that word that suggests it is a job configuration section, as opposed to server program, or even validation etc. (it all involves a "runtime").

@hjoliver
Copy link
Member Author

hjoliver commented Oct 9, 2019

        'job': {
            'execution retry delays': [VDR.V_INTERVAL_LIST, None],
            'execution time limit': [VDR.V_INTERVAL],
        },

@hjoliver
hjoliver 18 hours ago Member

Now that everything else has been moved out of the job section, I think we can move these two items up to the top level (next to script etc.).
@oliver-sanders
oliver-sanders 9 hours ago Member

Shouldn't the submission equivalents be here too?

@hjoliver
Copy link
Member Author

hjoliver commented Oct 9, 2019

        'outputs': {

@hjoliver
hjoliver 18 hours ago Member

task outputs? (to be clearer...) (or job outputs).

Actually: custom outputs? (these are after all, the user-defined outputs that can be triggered off, as opposed to the automatic ones like "started" and "succeeded").
@wxtim
wxtim 13 hours ago Author Member

This is another one where I think discussion might be sensible...
@oliver-sanders
oliver-sanders 9 hours ago Member

We might want to consider how exit code mapping might look in the future?

@matthewrmshin matthewrmshin added this to the cylc-8.0.0 milestone Oct 10, 2019
@dpmatthews
Copy link
Contributor

Superseded by #3422

@dpmatthews dpmatthews removed this from the cylc-8.0.0 milestone Nov 19, 2019
@dpmatthews dpmatthews added the duplicate This is a duplicate of something else label Nov 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This is a duplicate of something else
Projects
None yet
Development

No branches or pull requests

3 participants