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

HH-206678 use fastapi dependencies #683

Merged
merged 1 commit into from
Feb 17, 2024
Merged

HH-206678 use fastapi dependencies #683

merged 1 commit into from
Feb 17, 2024

Conversation

712u3
Copy link
Contributor

@712u3 712u3 commented Feb 14, 2024

@712u3 712u3 requested a review from a team as a code owner February 14, 2024 11:55
async def execute_page_method_with_dependencies(handler: Any, get_page_method: Callable) -> Any:
request = Request({
'type': 'http',
'query_string': '',
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

не очень хочу, это временная штука, когда в след итерации переедем нормально, то реквест будет сразу фастапишный

кстати обратил внимание, что я это добавлял чтобы сделать депенденсю get_current_handler, а потом забыл и сделал через contextvar, пару строчек выше, переделаю

'handler': handler,
})

route = next((rr for rr in handler.router.routes if rr.name == get_page_method.__name__ and rr.path == '/'), None)
Copy link
Contributor

Choose a reason for hiding this comment

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

не очень понятно, что тут происходит и почему ниже конкретный path = /base

Copy link
Contributor Author

@712u3 712u3 Feb 14, 2024

Choose a reason for hiding this comment

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

это костыль всия руси, мб будут получше предложения

в сервисе вот так бывает базированная страница

class BasePageHandler(FrontikPageHandler):
  router = ApiRouter(dependencies=[...])
  
  @router.get('/base', dependencies=[...])
  async def get_page(self):
      ...

страница наследница:

class DoughterPageHandler(BasePageHandler):
  router = ApiRouter(dependencies=BasePageHandler.router.dependencies, routes=BasePageHandler.router.routes)
  
  @router.get('/', dependencies=[...])
  async def post_page(self):
      ...

т.е. для этого дочернего класса хендлера у нас будет роутер, в котором нету get_page, и должен браться из родительского.
такчто в дочернем роутере добавляем routes

а может быть так что и в нашей есть get_page и в родительской, как гарантированно получить наш?

Copy link
Contributor

Choose a reason for hiding this comment

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

а может быть так что и в нашей есть get_page и в родительской, как гарантированно получить наш?

я не до конца понимаю ситуацию чето.

если ты в наследнике переопределил get_page - то всегда его и будешь получать?

Copy link
Contributor Author

@712u3 712u3 Feb 15, 2024

Choose a reason for hiding this comment

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

придумал как "нормально" сделать, обновил пр

def delete(self, **kwargs) -> Callable: # type: ignore
return super().delete('', **kwargs)

def api_route(self, *args, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

это удалится или останется после миграции?

Copy link
Contributor Author

@712u3 712u3 Feb 15, 2024

Choose a reason for hiding this comment

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

удалится, это чисто для промежуточного этапа, чтоб людей пока не путать с непонятными path

Copy link

hh-sonar bot commented Feb 16, 2024

SonarQube Quality Gate

Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@hhrelease hhrelease merged commit b96d7c8 into master Feb 17, 2024
2 checks passed
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