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

feat!: prep for rootPathFormat becoming ALL UPPERS #222

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

epmog
Copy link
Contributor

@epmog epmog commented Mar 21, 2024

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

API model for rootPathFormat is changing from lowercase to uppercase, so we need to prep ahead of time so that we support both when the swap happens.

What was the solution? (How)

  • Re-added the shim to deadline.client to ensure that create_job uses the correct version of rootPathFormat.
  • Modified deadline.job_attachments to switch the enum to ALL UPPERS, and if the enum can't be found (ie. windows) that it would see if the uppercase version exists.
  • get_job is also affected, but we only use this on download, so we can just swap that usage to ALL UPPERS all the time.
  • Downstream dependencies worker agent and openjd handle PathFormats regardless of lowers or uppers (they use uppers), so this doesn't actually break them.

What is the impact of this change?

  • If a model exists that uses lowercase enums, it will use that on create_job
  • Regardless of what model exists for get_job, it'll work.

How was this change tested?

Added tests around the _missing_ feature of the enum, as well as the DeadlineClient create_job shim

hatch run fmt
hatch build
hatch run lint
hatch run test

I then attempted to submit job bundles using the old model, and confirmed that it changed the request to use the lowercase version.

Was this change documented?

N/A

Is this a breaking change?

This is a breaking change since we're changing the PathFormat enum's values and the format of the strings returned for the path mapping rules in sync_inputs (even though the only known dependency handles this already).

jericht
jericht previously approved these changes Mar 21, 2024
@epmog epmog force-pushed the UPDATE_PATH_FORMAT branch 4 times, most recently from 604b693 to 64a53b7 Compare March 21, 2024 05:09
@epmog epmog marked this pull request as ready for review March 21, 2024 05:10
@epmog epmog requested a review from a team as a code owner March 21, 2024 05:10
Copy link
Contributor

@ddneilson ddneilson left a comment

Choose a reason for hiding this comment

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

We're sure that this one is changing? Things are moving quick, so it's not clear to me.

Copy link
Contributor

@marofke marofke left a comment

Choose a reason for hiding this comment

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

🚢 🦈

** BREAKING CHANGE **
* The PathFormat enum's values went from all lowercase to all uppercase
* The source_path_root in the path mapping rules return value from sync_inputs went from all lowercase to all uppercase

Signed-off-by: Morgan Epp <60796713+epmog@users.noreply.github.com>
@epmog epmog merged commit d49c885 into mainline Mar 21, 2024
18 checks passed
@epmog epmog deleted the UPDATE_PATH_FORMAT branch March 21, 2024 19:38
baxeaz pushed a commit that referenced this pull request Mar 21, 2024
** BREAKING CHANGE **
* The PathFormat enum's values went from all lowercase to all uppercase
* The source_path_root in the path mapping rules return value from sync_inputs went from all lowercase to all uppercase

Signed-off-by: Morgan Epp <60796713+epmog@users.noreply.github.com>
Signed-off-by: Brian Axelson <baxelson@amazon.com>
baxeaz added a commit that referenced this pull request Mar 22, 2024
* Switch to running deadline_vfs as os_user

Signed-off-by: Brian Axelson <baxelson@amazon.com>

* feat(job_attachments): enhance handling S3 timeout errors and BotoCoreError (#206)

Improve error handling for S3 requests by
- adding "retries" configuration to the S3 client
- adding BotoCoreError handling to cover S3 timeout errors (e.g., ReadTimeoutError, ConnectTimeoutError)

Signed-off-by: Gahyun Suh <132245153+gahyusuh@users.noreply.github.com>
Signed-off-by: Brian Axelson <baxelson@amazon.com>

* fix(job_attachments): Use files' last modification time to identify output files to be synced (#211)

Signed-off-by: Gahyun Suh <132245153+gahyusuh@users.noreply.github.com>
Signed-off-by: Brian Axelson <baxelson@amazon.com>

* chore(deps): update python-semantic-release requirement (#216)

Updates the requirements on [python-semantic-release](https://github.com/python-semantic-release/python-semantic-release) to permit the latest version.
- [Release notes](https://github.com/python-semantic-release/python-semantic-release/releases)
- [Changelog](https://github.com/python-semantic-release/python-semantic-release/blob/master/CHANGELOG.md)
- [Commits](python-semantic-release/python-semantic-release@v8.7.0...v9.2.2)

---
updated-dependencies:
- dependency-name: python-semantic-release
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Brian Axelson <baxelson@amazon.com>

* chore(release): 0.41.0 (#217)

Signed-off-by: client-software-ci <129794699+client-software-ci@users.noreply.github.com>
Signed-off-by: Brian Axelson <baxelson@amazon.com>

* chore(deps): update coverage[toml] requirement from ~=7.2 to ~=7.4 (#156)

Updates the requirements on [coverage[toml]](https://github.com/nedbat/coveragepy) to permit the latest version.
- [Release notes](https://github.com/nedbat/coveragepy/releases)
- [Changelog](https://github.com/nedbat/coveragepy/blob/master/CHANGES.rst)
- [Commits](nedbat/coveragepy@7.3.0...7.4.0)

---
updated-dependencies:
- dependency-name: coverage[toml]
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Brian Axelson <baxelson@amazon.com>

* fix: Make StorageProfileOperatingSystemFamily enum case-insensitive

Signed-off-by: Caden Marofke <marofke@amazon.com>
Signed-off-by: Brian Axelson <baxelson@amazon.com>

* ci: add gpg signing of build artifacts (#218)

Signed-off-by: Charles Moore <122481442+moorec-aws@users.noreply.github.com>
Signed-off-by: Brian Axelson <baxelson@amazon.com>

* feat!: prep for rootPathFormat becoming ALL UPPERS (#222)

** BREAKING CHANGE **
* The PathFormat enum's values went from all lowercase to all uppercase
* The source_path_root in the path mapping rules return value from sync_inputs went from all lowercase to all uppercase

Signed-off-by: Morgan Epp <60796713+epmog@users.noreply.github.com>
Signed-off-by: Brian Axelson <baxelson@amazon.com>

* CR Feedback

Signed-off-by: Brian Axelson <baxelson@amazon.com>

* Cleaning up a few more 'executing the job' cases

Signed-off-by: Brian Axelson <baxelson@amazon.com>

---------

Signed-off-by: Brian Axelson <baxelson@amazon.com>
Signed-off-by: Gahyun Suh <132245153+gahyusuh@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: client-software-ci <129794699+client-software-ci@users.noreply.github.com>
Signed-off-by: Caden Marofke <marofke@amazon.com>
Signed-off-by: Charles Moore <122481442+moorec-aws@users.noreply.github.com>
Signed-off-by: Morgan Epp <60796713+epmog@users.noreply.github.com>
Co-authored-by: Gahyun Suh <132245153+gahyusuh@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: client-software-ci <129794699+client-software-ci@users.noreply.github.com>
Co-authored-by: Caden Marofke <marofke@amazon.com>
Co-authored-by: Charles Moore <122481442+moorec-aws@users.noreply.github.com>
Co-authored-by: Morgan Epp <60796713+epmog@users.noreply.github.com>
epmog added a commit that referenced this pull request Mar 25, 2024
epmog added a commit that referenced this pull request Mar 25, 2024
This reverts commit d49c885.

Signed-off-by: Morgan Epp <60796713+epmog@users.noreply.github.com>
epmog added a commit that referenced this pull request Mar 25, 2024
…243)

This reverts commit d49c885.

Signed-off-by: Morgan Epp <60796713+epmog@users.noreply.github.com>
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.

4 participants