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

info: only resolve workspaceIdentity, workspaceProject as needed #161

Merged
merged 2 commits into from
Feb 8, 2022

Conversation

wwade
Copy link
Owner

@wwade wwade commented Feb 8, 2022

The plugins' implementation of these methods could be somewhat expensive, so we don't
want to, for example, call workspaceProject() each time a JobInfo is initialized as
part of reading a job entry from the database.

To address this, we call resolve() to resolve the attributes' values in only two
instances:

  1. when the job is created (but not yet started), and
  2. when the job is started.

For the case of (2), resolve(force=True) is used so that we override the creation
values in case something has changed in the environment that might affect these values.

Added partial typing information. This is just a start to allow more easily
navigating the code with a type-aware IDE.

After adding some typing info, resolve related errors as reported by pyright.
The plugins' implementation of these methods could be somewhat expensive, so we don't
want to, for example, call workspaceProject() each time a JobInfo is initialized as
part of reading a job entry from the database.

To address this, we call resolve() to resolve the attributes' values in only two
instances:
  1. when the job is created (but not yet started), and
  2. when the job is started.

For the case of (2), `resolve(force=True)` is used so that we override the creation
values in case something has changed in the environment that might affect these values.
@wwade wwade enabled auto-merge February 8, 2022 00:13
@wwade wwade merged commit 91ad1be into master Feb 8, 2022
@wwade wwade deleted the plug-defer branch February 8, 2022 00:14
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.

None yet

1 participant