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

Restore task element name / element name distinction in UI #1428

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Cynical-Optimist
Copy link

See original merge request on GitLab
In GitLab by [Gitlab user @tristanvb] on Sep 21, 2020, 10:05

This behavior has regressed a while back when introducing the messenger
object in 0026e379 from merge request !1500.

Main behavior change:

  - Messages in the master log always appear with the task element's
    element name and cache key, even if the element or plugin issuing
    the log line is not the primary task element.

  - Messages logged in the task specific log, retain the context of the
    element names and cache keys which are issuing the log lines.

Changes include:

  * _message.py: Added the task element name & key members

  * _messenger.py: Log the element key as well if it is provided

  * _widget.py: Prefer the task name & key when logging, we fallback
    to the element name & key in case messages are being logged outside
    of any ongoing task (main process/context)

  * job.py: Unconditionally stamp messages with the task name & key

    Also removed some unused parameters here, clearing up an XXX comment

  * plugin.py: Make _message() internal so that element.py can override it

  * element.py: Override _message() so that we can add the issuing element's
    cache key to the message context.

Fixes #1393

Tristan van Berkom added 6 commits October 1, 2020 17:01
Instead of passing around untyped tuples for cache keys, lets have
a clearly typed object for this.

This makes for more readable code, and additionally corrects the
data model statement of intent that some cache keys should be displayed
as "dim", instead informing the frontend about whether the cache key
is "strict" or not, allowing the frontend to decide how to display
a strict or non-strict key.

This patch does the following:

  * types.py: Add _DisplayKey

  * element.py: Return a _DisplayKey from Element._get_display_key()

  * Other sources: Updated to use the display key object
Instead of using Element._get_brief_display_key(), no need for that
second accessor.
Now that we have _DisplayKey(), no need for this extra accessor, it's
not helping anything to have it anymore.
This allows more flexible usage of the APIs, to allow passing all of the
Message() constructor arguments through kwargs.
This behavior has regressed a while back when introducing the messenger
object in 0026e37 from merge request !1500.

Main behavior change:

  - Messages in the master log always appear with the task element's
    element name and cache key, even if the element or plugin issuing
    the log line is not the primary task element.

  - Messages logged in the task specific log, retain the context of the
    element names and cache keys which are issuing the log lines.

Changes include:

  * _message.py: Added the task element name & key members

  * _messenger.py: Log the element key as well if it is provided

  * _widget.py: Prefer the task name & key when logging, we fallback
    to the element name & key in case messages are being logged outside
    of any ongoing task (main process/context)

  * job.py: Unconditionally stamp messages with the task name & key

    Also removed some unused parameters here, clearing up an XXX comment

  * plugin.py: Add new _get_message_kwargs() method to collect the kwargs
               appropriate to construct Messages() issued by the plugin.

    Use this method to construct messages in Plugin.__message() and to
    pass kwargs along to Messenger.timed_activity().

  * element.py: Override _get_message_kwargs() so as to provide the
                cache key in messages issued by this element.

  * tests/frontend/logging.py: Fix test to expect the cache key in the logline

  * tests/frontend/artifact_log.py: Fix test to expect the cache key in the logline

Fixes #1393
@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @tristanvb] on Sep 23, 2020, 07:43

added 13 commits

  • 0ac455d1...ee43d10c - 2 commits from branch master
  • 14377d4 - Element: Substitute variables on dependency configurations
  • 3790e90 - Sandbox: Assert absolute paths where they are set
  • be7d81b - ArtifactElement: Don't call Sandbox.set_output_directory()
  • b792b12 - Sandbox: Remove Sandbox.set_output_directory()
  • 50e2cdfc - .gitlab-ci.yml: Use ported plugins and fdsdk for overnight tests
  • 45fd3117 - Adding _DisplayKey type
  • b98c2bdb - _elementsources.py: Fix copy/paste error in API doc comment
  • 7a66916f - _artifactcache.py: Use Element._get_display_key()
  • e2b5282b - element.py: Remove Element._get_brief_display_key()
  • 2085c755 - _messenger.py: Use kwargs in timed_activity() and simple_task()
  • 4ee9df9a - Restore task element name / element name distinction in UI

Compare with previous version

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @tristanvb] on Sep 23, 2020, 11:39

added 8 commits

  • 4ee9df9a...34d69a5b - 2 commits from branch master
  • fbb69622 - Adding _DisplayKey type
  • 39095455 - _elementsources.py: Fix copy/paste error in API doc comment
  • 0e0e4c6e - _artifactcache.py: Use Element._get_display_key()
  • c72197b0 - element.py: Remove Element._get_brief_display_key()
  • 73f0afa9 - _messenger.py: Use kwargs in timed_activity() and simple_task()
  • 28762fe5 - Restore task element name / element name distinction in UI

Compare with previous version

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @tristanvb] on Sep 24, 2020, 12:13

added 11 commits

  • 28762fe5...9fba7a1f - 5 commits from branch master
  • 56f425e7 - Adding _DisplayKey type
  • 8308d557 - _elementsources.py: Fix copy/paste error in API doc comment
  • d0071e1b - _artifactcache.py: Use Element._get_display_key()
  • e9cc9ea1 - element.py: Remove Element._get_brief_display_key()
  • 928cf255 - _messenger.py: Use kwargs in timed_activity() and simple_task()
  • e399c00d - Restore task element name / element name distinction in UI

Compare with previous version

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @tristanvb] on Sep 25, 2020, 08:02

added 9 commits

  • e399c00d...bb317336 - 3 commits from branch master
  • 6ad4f85e - Adding _DisplayKey type
  • ea85ac25 - _elementsources.py: Fix copy/paste error in API doc comment
  • 8e3978f7 - _artifactcache.py: Use Element._get_display_key()
  • 39388e41 - element.py: Remove Element._get_brief_display_key()
  • 6c4b7491 - _messenger.py: Use kwargs in timed_activity() and simple_task()
  • 136e8699 - Restore task element name / element name distinction in UI

Compare with previous version

@Cynical-Optimist
Copy link
Author

In GitLab by [Gitlab user @tristanvb] on Oct 1, 2020, 09:02

added 26 commits

  • 136e8699...ea235b9d - 20 commits from branch master
  • 2c5fbf8 - Adding _DisplayKey type
  • 955e050 - _elementsources.py: Fix copy/paste error in API doc comment
  • 0236a28 - _artifactcache.py: Use Element._get_display_key()
  • bf3a2b7 - element.py: Remove Element._get_brief_display_key()
  • e09b7c0 - _messenger.py: Use kwargs in timed_activity() and simple_task()
  • 4d6a9f7 - Restore task element name / element name distinction in UI

Compare with previous version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant