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

Group Attributes of Class and Object elements of Google Wallet, to make it more maintainable. #6

Merged
merged 38 commits into from
Dec 3, 2024

Conversation

loechel
Copy link
Contributor

@loechel loechel commented Apr 22, 2024

Group Attributes of Class and Object elements of Google Wallet to make it mor maintainable.

)
if path.exists():
self._credentials_file = path
return self._credentials_file

Copy link
Contributor

Choose a reason for hiding this comment

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

This has to be documented.

@@ -394,9 +400,9 @@ def save_link(
}
signer = crypt.RSASigner.from_service_account_file(session_manager.credentials_file)
jwt_string = jwt.encode(signer, claims).decode("utf-8")
logger.debug(
"JWT-Length: %d, is less than recommenden 1800: %s",
logger.warning(
Copy link
Contributor

@jensens jensens Apr 26, 2024

Choose a reason for hiding this comment

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

I am not sure if this is worth a warning. It works anyway if it is longer.

@jensens
Copy link
Contributor

jensens commented Apr 26, 2024

This needs also:

  • major version bump.
  • change log entry
  • updates in documentation repository.

@@ -170,6 +170,8 @@ def update(
url=session_manager.url(name, f"/{resource_id}"),
data=verified_json.encode("utf-8"),
)
logger.debug(verified_json.encode("utf-8"))
# print(verified_json.encode("utf-8"))
Copy link
Contributor

Choose a reason for hiding this comment

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

This commented print should go before merge.

@@ -12,7 +12,7 @@ class GoogleWalletModel(BaseModel):
"""

model_config = ConfigDict(
# extra="forbid",
extra="forbid",
# extra="ignore",
# use_enum_values=True,
Copy link
Contributor

Choose a reason for hiding this comment

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

@loechel
Copy link
Contributor Author

loechel commented Apr 29, 2024

@simon-lund please make a detailed review and if possible also make additional commits.

This Pull-Request changes the default behavior of eduTAP.wallet_google only to accept model attributes to be set if they are explicitly in the model. This follows PEP 20 Zen of Python: explicit is better than implicit

Each model element should have a reference to its documentation and all relevant attributes.
Please check if all attributes are modeled.

Co-authored-by: Jens W. Klein <jk@kleinundpartner.at>
@loechel loechel requested review from jensens, zworkb and simon-lund May 1, 2024 17:52
@jensens
Copy link
Contributor

jensens commented May 2, 2024

Before merging, a PR to update https://github.com/edutap-eu/documentation/tree/main/source/packages/edutap_wallet_google is needed over there.

@jensens jensens mentioned this pull request Nov 29, 2024
@jensens jensens merged commit 6bf4ec9 into main Dec 3, 2024
6 checks passed
@jensens jensens deleted the group_attributes branch February 7, 2025 00:47
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.

2 participants