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

PRMP-640 #11

Merged
merged 8 commits into from
Aug 16, 2024
Merged

PRMP-640 #11

merged 8 commits into from
Aug 16, 2024

Conversation

NogaNHS
Copy link
Contributor

@NogaNHS NogaNHS commented Aug 5, 2024

No description provided.

@NogaNHS NogaNHS marked this pull request as ready for review August 6, 2024 13:56
gp_dynamo_item.update(
actions=[
PracticeOds.supplier_name.set(requesting_supplier_name),
PracticeOds.supplier_last_updated.set(datetime.now()),

Choose a reason for hiding this comment

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

Could we check whether the supplier_last_updated should be of date type or datetime type?
I see that we use date.today() above (which output date type), and we have datetime.now() here (which output datetime type) .
When I tried compare date and datetime in local terminal it gave an error:

>>> datetime.now() < date.today()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: can't compare datetime.datetime to datetime.date

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you try datetime.now().date() < date.today() it should be ok.
We convert datetime to date in line 179 😄

if supplier_name is None:

if supplier_name is None:
print(
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we able to add logger here instead of print?

@NogaNHS NogaNHS changed the base branch from master to PRMP-594 August 16, 2024 13:54
@NogaNHS NogaNHS merged commit 32b1d01 into PRMP-594 Aug 16, 2024
1 check passed
@chrisbloe-nhse chrisbloe-nhse deleted the PRMP-640 branch September 26, 2024 12:02
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