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

Oai organization #2

Merged
merged 29 commits into from
Nov 13, 2024
Merged

Oai organization #2

merged 29 commits into from
Nov 13, 2024

Conversation

kyokukou
Copy link
Contributor

@kyokukou kyokukou commented Nov 6, 2024

general file system layout, started to create separate paths for different verbs and errors

@kyokukou kyokukou requested a review from bdc34 November 6, 2024 18:15
Copy link

Choose a reason for hiding this comment

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

Maybe rename this macros.xml?

if from_str:
try:
if not re.fullmatch(DATE_REGEX, from_str):
raise ValueError
Copy link

Choose a reason for hiding this comment

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

This is redundant since strptime will throw a ValueError

Copy link
Contributor Author

Choose a reason for hiding this comment

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

strptime is a little too permissive and accepts things like 3-2-2024 which the specification says must be 03-02-2024

#TODO duplicate params dont create errors, technically not to spec
params: Dict[str, str] = request.args.to_dict() if request.method == 'GET' else request.form.to_dict()

response, code, headers=verb_sorter(params)
Copy link

Choose a reason for hiding this comment

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

Could the verb_sorter function be removed and the match statement moved here?


return "<a>b</a>", 200, {}

def list_records(params: Dict[str, str]) -> Response:
Copy link

Choose a reason for hiding this comment

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

list_records and list_identifiers may be unnecessary extra level of function calls.

#TODO
return "<a>b</a>", 200, {}

def list_metadata_formats(params: Dict[str, str]) -> Response:
Copy link

Choose a reason for hiding this comment

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

I could see this expected params processing split out to a function.

Copy link

@bdc34 bdc34 left a comment

Choose a reason for hiding this comment

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

Some minor comments. Overall looks good.

@kyokukou kyokukou added this pull request to the merge queue Nov 13, 2024
Merged via the queue into develop with commit 7da91c0 Nov 13, 2024
3 checks passed
@kyokukou kyokukou deleted the oai-organization branch November 13, 2024 16:05
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