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

added metadata section to platforms and platform groups config #4558

Merged
merged 4 commits into from
Jan 13, 2022

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Dec 17, 2021

These changes close #4528

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg and conda-environment.yml.
  • Does not need tests (Config change).
  • Appropriate change log entry included.
  • No documentation update required, yet. See cylc config --list-platforms #4557

@wxtim wxtim self-assigned this Dec 17, 2021
@wxtim wxtim added this to the cylc-8.0.0 milestone Dec 17, 2021
@wxtim wxtim requested a review from oliver-sanders December 17, 2021 13:37
@wxtim wxtim added the small label Dec 17, 2021
Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

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

Looks good. Just wondering about a few things though:

  • worth putting the common descriptive text in a string constant?
  • would a short-cut option would be useful (e.g. --describe-platforms) that just prints platform names and metadata?
  • there's no internal use for this, so I wonder if a single meta (or description?) item would suffice, rather than a section for multiple arbitrary items? (I suppose it's possible that individual items that can be extracted separately could be useful at a site ... e.g. cost per cpu hour or something?).

@wxtim
Copy link
Member Author

wxtim commented Dec 20, 2021

* worth putting the common descriptive text in a string constant?

Yes

* would a short-cut option would be useful (e.g. `--describe-platforms`) that just prints platform names and metadata?

I think so; see #4557 - I'm think that I'm going to take your comment as a yes to the question in that ticket.

* there's no internal use for this, so I wonder if a single `meta` (or `description`?) item would suffice, rather than a section for multiple arbitrary items?  (I suppose it's possible that individual items that can be extracted separately could be useful at a site ... e.g. `cost per cpu hour` or something?).

It would, but I think it's reasonable to leave it open ended - examples might include

[meta]
description = """
Our Cluster.
"""
url=http://somedocs.internalwebsite.com
max-memory-request=200G
max-cpu-request=12

Though to be honest, I'm not completely convinced that this is required.

@wxtim wxtim requested a review from hjoliver January 4, 2022 10:39
@wxtim
Copy link
Member Author

wxtim commented Jan 12, 2022

@hjoliver : I think I've now

  • added the description field.
  • justified my decision to leave other fields open ended[1].
  • Created a new ticket to discuss the platform listing.

[1] Just because Cylc has no internal use for formally defined fields doesn't mean that someone else can't use them. For example, my personal workflows repo has a script which GH actions uses to extract metadata and dump it in the repo README.md. I could put the formal fields I want into a single description box and parse them in the same way, but it seems nicer to have them as metadata in the config.

@MetRonnie MetRonnie modified the milestones: cylc-8.0.0, cylc-8.0rc1 Jan 12, 2022
@hjoliver
Copy link
Member

added the description field.

I think you might have misinterpreted my comment on this, which was:

worth putting the common descriptive text in a string constant?

I meant, this descriptive text is identical for platform and group:

                Allows writers of platform configurations to add information
                about platform usage. There are no-preset items because
                Cylc does not use any platform metadata internally.

                Users can then see information about defined platforms using::

                   cylc config -i [platforms]

                .. seealso::
                   :ref:`AdminGuide.PlatformConfigs`

... so, it should really be stored in a string constant to avoid the duplication.

Also, There are no-preset items is now not true since you've added a preset description item (which I don't think we need, if going with the arbitrary metadata solution).

@wxtim
Copy link
Member Author

wxtim commented Jan 12, 2022

@hjoliver - completely got the wrong end of the stick here - have pushed another commit which hopefully hits nearer the mark.

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

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

That's got it, thanks @wxtim

@oliver-sanders oliver-sanders merged commit 1be6ede into cylc:master Jan 13, 2022
@wxtim wxtim deleted the add_platform_metadata branch March 22, 2022 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Metadata to Platforms
4 participants