Skip to content

Conversation

@ogjaylowe
Copy link
Contributor

Description

Referenced self.name instead of self.__project_name. During original testing these coincidently were the same but can sometimes differ when we do string sanitation.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

@FrancescoSaverioZuppichini
Copy link
Contributor

Hi @ogjaylowe , thanks for the fix. Can you describe the issue better? Usually (even if there is badly used), __ notation means "this is very internal". What self.name means and what self.__project_name means?

Without a clear understanding of this, I cannot accept the changes. Sorry for the waiting, and thanks again for the PR

@ogjaylowe ogjaylowe requested a review from hansent February 27, 2023 16:16
@hansent
Copy link
Contributor

hansent commented Feb 27, 2023

ah, I actually ran into the same error while working on something else (but committed it to Jacobs Quickstart branch as it was part of that.)

Definitly a bug in that it should use the project_url instead of the project_name

@ogjaylowe Do you know if self.__project_name is always the project_url? The fix for it on the other branch used the parameter being passed in (split by /) similar to how its done in the upload function: 9cdf23c

@hansent
Copy link
Contributor

hansent commented Feb 27, 2023

I checked and self.__project_name is the same as id.rsplit("/")[1]

Copy link
Contributor

@paulguerrie paulguerrie left a comment

Choose a reason for hiding this comment

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

Seems legit

@hansent hansent merged commit 34cb8c8 into main Feb 27, 2023
@hansent hansent deleted the upload-annotation-project-name-fix branch February 27, 2023 19:18
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.

5 participants