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

Updating argo-wrapper dependencies #151

Merged
merged 23 commits into from
Nov 11, 2024
Merged

Updating argo-wrapper dependencies #151

merged 23 commits into from
Nov 11, 2024

Conversation

m0nhawk
Copy link
Contributor

@m0nhawk m0nhawk commented Oct 22, 2024

Link to JIRA ticket if there is one: VADC-1508

New Features

Breaking Changes

Bug Fixes

Improvements

Dependency updates

  • updating dependencies

Deployment changes

pyproject.toml Outdated Show resolved Hide resolved
tianj7
tianj7 previously approved these changes Oct 31, 2024
pieterlukasse
pieterlukasse previously approved these changes Nov 4, 2024
@@ -405,7 +418,7 @@ def get_workflows_for_team_project(self, team_project: str) -> List[Dict]:
workflows = self.get_workflows_for_label_selector(label_selector=label_selector)
return workflows

def get_workflows_for_user(self, auth_header: str) -> List[Dict]:
def get_workflows_for_user(self, auth_header: Optional[str]) -> List[Dict]:
Copy link
Contributor

@pieterlukasse pieterlukasse Nov 6, 2024

Choose a reason for hiding this comment

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

Suggested change
def get_workflows_for_user(self, auth_header: Optional[str]) -> List[Dict]:
def get_workflows_for_user(self, auth_header: str) -> List[Dict]:

I don't think auth_header is optional here, and probably also not optional in most other places where it is now marked as optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's because of this, the get can return None, so it expects it to be Optional:

auth_header=request.headers.get("Authorization"),

In short, we need better type annotations.

@m0nhawk m0nhawk requested a review from pieterlukasse November 7, 2024 06:50
pieterlukasse
pieterlukasse previously approved these changes Nov 8, 2024
@m0nhawk m0nhawk merged commit 5308586 into master Nov 11, 2024
9 of 10 checks passed
@m0nhawk m0nhawk deleted the dep/update branch November 11, 2024 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants