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

chore(api)!: rename jobsRunAs to jobRunAsUser #41

Merged
merged 1 commit into from
Sep 28, 2023
Merged

Conversation

epmog
Copy link
Contributor

@epmog epmog commented Sep 25, 2023

What was the problem/requirement? (What/Why)

The jobsRunAs property in the API is being renamed in the service to jobRunAsUser, so the worker agent needs to follow suit.

What was the solution? (How)

The Worker agent uses the following priority order when reading the serivce response:

  1. jobRunAsUser if it exists in the api response, otherwise
  2. jobsRunAs, otherwise
  3. None

What is the impact of this change?

Worker Agent works with new API shape.

How was this change tested?

hatch run fmt
hatch run lint
hatch build
hatch run test

Was this change documented?

N/A

Is this a breaking change?

Yes.

@epmog epmog force-pushed the rename_jobs_run_as branch 7 times, most recently from 807291f to 075a57a Compare September 25, 2023 21:12
@epmog epmog marked this pull request as ready for review September 25, 2023 21:13
@epmog epmog requested a review from a team as a code owner September 25, 2023 21:13
@epmog epmog force-pushed the rename_jobs_run_as branch 2 times, most recently from 058709d to 8eda08a Compare September 25, 2023 22:29
Comment on lines +192 to +195
# Use jobRunAsUser if it exists, otherwise jobsRunAs if it exists, otherwise None.
job_run_as_user_data = job_details_data.get(
"jobRunAsUser", job_details_data.get("jobsRunAs", None)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: should we maybe put a comment like TODO: remove this workaround once service no longer sends jobsRunAs here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll be doing a follow-up to remove all jobsRunAs references once the service swaps (dependent on this), so I should catch this reference.

@epmog epmog force-pushed the rename_jobs_run_as branch from 8eda08a to 4999474 Compare September 28, 2023 19:05
@epmog epmog merged commit c34a3af into mainline Sep 28, 2023
8 checks passed
@epmog epmog deleted the rename_jobs_run_as branch September 28, 2023 19:11
gmchale79 pushed a commit that referenced this pull request Oct 31, 2023
Signed-off-by: Morgan Epp <[email protected]>
Signed-off-by: Graeme McHale <[email protected]>
gmchale79 pushed a commit that referenced this pull request Nov 2, 2023
Signed-off-by: Morgan Epp <[email protected]>
Signed-off-by: Graeme McHale <[email protected]>
gahyusuh pushed a commit that referenced this pull request Nov 6, 2023
Signed-off-by: Morgan Epp <[email protected]>
Signed-off-by: Graeme McHale <[email protected]>
Signed-off-by: Gahyun Suh <[email protected]>
gmchale79 pushed a commit that referenced this pull request Feb 12, 2024
Signed-off-by: Morgan Epp <[email protected]>
Signed-off-by: Graeme McHale <[email protected]>
gmchale79 pushed a commit that referenced this pull request Mar 11, 2024
Signed-off-by: Morgan Epp <[email protected]>
Signed-off-by: Graeme McHale <[email protected]>
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.

3 participants