Skip to content

Conversation

@edwards-aws
Copy link
Contributor

Fixes: #179

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

See https://github.com/OpenJobDescription/openjd-adaptor-runtime-for-python/actions/runs/14341779725/job/40202541223?pr=179

A mypy update detected some issues.

What was the solution? (How)

Be more precise in the code with typing. There were a couple function calls that had mis-matched types.

In model.py reassert what we know to be true about the type.

In _configuration_manager.py the type in the function argument was actually incorrect. We wanted the class type, not an instance.

I also found a kind of bug in DataclassMapper. We were never actually reaching the code that maps an enum value to an enum. Now in practice this didn't matter because we were using string enums and the underlying data we were checking is the same whether or not the value is an enum. However this wasn't the intent of the code, we wanted an accurate mapping to the enum.

The issue was that we were checking if the field was a subclass of Enum. The typing stuff gets kinda confusing here, the enum field type is actually EnumMeta, which isn't a subclass of Enum.

It's called EnumType in Python >= 3.11, we'll use EnumMeta for backward compatibility as EnumMeta is an alias for EnumType in Python >= 3.11

So now we check if the field is an EnumMeta. The code afterwords could be simplified quite a bit after we know that it's an Enum as well.

What is the impact of this change?

More accurate typing, hopefully more robust code.

How was this change tested?

hatch run test

I also added tests for mapping Enums as these were missing before.

  • Have you run the unit tests?
    Yep

Was this change documented?

Nothing to document.

  • Are relevant docstrings in the code base updated?
    Ya, this doesn't impact any of the public APIs.

Is this a breaking change?

No

Does this change impact security?

No

  • Does the change need to be threat modeled? For example, does it create or modify files/directories that must only be readable by the process owner?
    No

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@edwards-aws edwards-aws requested a review from a team as a code owner April 9, 2025 21:48
@sonarqubecloud
Copy link

@edwards-aws edwards-aws merged commit 2b3918f into OpenJobDescription:mainline Apr 11, 2025
19 checks passed
jblagden pushed a commit to jblagden/openjd-adaptor-runtime-for-python that referenced this pull request Aug 21, 2025
Signed-off-by: Cody Edwards <[email protected]>
Signed-off-by: Justin Blagden <[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