Allow custom models in headlines and descriptions#15
Conversation
… headline and description generation separately
JuanGriggio
left a comment
JuanGriggio
left a comment
There was a problem hiding this comment.
Ale, deje unos comments, si queres después lo vemos
| self.client_id=None | ||
| self.client_secret=None | ||
| self.refresh_token=None |
There was a problem hiding this comment.
Estas 3 lineas no hacen falta
| client_id = config.get('client_id') | ||
| client_secret = config.get('client_secret') | ||
| refresh_token = config.get('refresh_token') | ||
| self.client_id = config.get('client_id') | ||
| self.client_secret = config.get('client_secret') | ||
| self.refresh_token = config.get('refresh_token') |
There was a problem hiding this comment.
No hace falta que sean variables de instancia (no hace falta el self.)
| self.refresh_token = config.get('refresh_token') | ||
|
|
||
| if not client_id or not client_secret or not refresh_token: | ||
| if not self.client_id or not self.client_secret or not self.refresh_token: |
There was a problem hiding this comment.
No hace falta que sean variables de instancia (no hace falta el self.)
| 'client_id': client_id, | ||
| 'client_secret': client_secret, | ||
| 'refresh_token': refresh_token, | ||
| 'client_id': self.client_id, | ||
| 'client_secret': self.client_secret, | ||
| 'refresh_token': self.refresh_token, |
There was a problem hiding this comment.
No hace falta que sean variables de instancia (no hace falta el self.)
| if not client_id or not client_secret or not refresh_token: | ||
| if not self.client_id or not self.client_secret or not self.refresh_token: | ||
| creds, _ = default(scopes=API_SCOPES) | ||
| self.is_authentication_method_client_id=False |
| self.client_id=None | ||
| self.client_secret=None | ||
| self.refresh_token=None | ||
| self.is_authentication_method_client_id='unauthenticated' |
There was a problem hiding this comment.
Mejor inicializarlo así: self.is_authentication_method_client_id=False o self.is_authentication_method_client_id=None. De esta forma, la variable es de un solo tipo (bool)
There was a problem hiding this comment.
En general la idea está muy bien, pero yo no escribiría todo el código en este archivo. El ContentGeneratorService no debería saber a qué endpoint le tiene que pegar, ni encargarse de temas de autenticación. Para eso están los AuthenticationHelper y GeminiHelper. Trataría de reestructurarlo de manera que cada servicio se ocupe de su propósito y nada más.
| response = self.model.generate_content( | ||
| prompt, | ||
| generation_config=GENERATION_CONFIG, | ||
| safety_settings=SAFETY_SETTINGS | ||
| ) | ||
| start_idx = response.text.index('[') | ||
| end_idx = response.text.index(']') | ||
| time.sleep(TIME_INTERVAL_BETWEEN_REQUESTS) | ||
|
|
||
| return ast.literal_eval(response.text[start_idx:end_idx+1]) | ||
| if custom_endpoint: | ||
| # Custom endpoint request | ||
| payload = { | ||
| "contents": { | ||
| "role": "USER", | ||
| "parts": {"text": prompt} | ||
| }, | ||
| "generation_config": GENERATION_CONFIG | ||
| } | ||
| headers = { | ||
| "Authorization": f"Bearer {access_token}", | ||
| "Content-Type": "application/json" | ||
| } | ||
| response = requests.post(custom_endpoint, json=payload, headers=headers) | ||
|
|
||
|
|
||
| if response.status_code != 200: | ||
| raise Exception(f"Custom endpoint request failed with status code {response.status_code}") | ||
| response_data = response.json() | ||
| response_data = response_data['candidates'][0]['content']['parts'][0]['text'] | ||
| start_idx = response_data.index('[') | ||
| end_idx = response_data.index(']') | ||
| return ast.literal_eval(response_data[start_idx:end_idx+1]) | ||
| else: | ||
| response = self.model.generate_content( | ||
| prompt, | ||
| generation_config=GENERATION_CONFIG, | ||
| safety_settings=SAFETY_SETTINGS | ||
| ) | ||
| start_idx = response.text.index('[') | ||
| end_idx = response.text.index(']') | ||
| time.sleep(TIME_INTERVAL_BETWEEN_REQUESTS) | ||
| return ast.literal_eval(response.text[start_idx:end_idx+1]) |
There was a problem hiding this comment.
Todo esto está muy bien, pero lo haría un poco diferente. En una función aparte, supongamos __call_gemini, haría que se llame al endpoint personalizado o al estándar según la configuración y retorne siempre lo mismo. De esta manera, el código de esta parte queda casi igual que antes, no se repite el procesado de la respuesta de gemini y es más prolijo
2 participants
Allow custom models in headlines and descriptions