Igor Platonov. Task 3. Actual #37
Open
+866
−33
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spajic
approved these changes
Aug 6, 2020
| file.rewind | ||
|
|
||
| @io = | ||
| if first_bytes == GZIP_BYTES # Проверяем, если GZIP, то умеем работать еще и с архивами |
| services << allowed_services[service] ||= Service.create!(name: service) | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Хорошо получилось, кратенько, понятно и производительно, ещё и с возможностью не распаковывать архив 👍
|
|
||
| По автобусам - можно было еще проводить проверку, что если есть такой number, то пропускать импорт такой записи. А по городам можно было убрать проверку на наличие пробелов. | ||
|
|
||
| Если мои способы(добавление уникальности по двум полям и избавление пробелов у всех городов) решения этих моментов не являются корректными, то я изменю это, например, на вариант с разрешением городов с пробелами и проверка number автобусов перед их созданием (либо ловить exception уникальности) |
There was a problem hiding this comment.
Способы ок. Валидации я добавил в основном для подвоха с тем, что они очень замедлят работу импорта, если импортировать по одной записи и каждую валидировать.
| По автобусам - можно было еще проводить проверку, что если есть такой number, то пропускать импорт такой записи. А по городам можно было убрать проверку на наличие пробелов. | ||
|
|
||
| Если мои способы(добавление уникальности по двум полям и избавление пробелов у всех городов) решения этих моментов не являются корректными, то я изменю это, например, на вариант с разрешением городов с пробелами и проверка number автобусов перед их созданием (либо ловить exception уникальности) | ||
| 2. Содержимое rake таски я перевел в service. Это позволяет удобнее тестировать, а также возможность использовать в нескольких места, если такое потребуется. Плюс так удобнее запускать из консоли и дебажить |
There was a problem hiding this comment.
++ всячески поддерживаю, рейк-таски как и фоновые задачи предпочитаю держать минималистичными - просто получить простые аргументы и вызывать сервис с этими аргументами.
| Если мои способы(добавление уникальности по двум полям и избавление пробелов у всех городов) решения этих моментов не являются корректными, то я изменю это, например, на вариант с разрешением городов с пробелами и проверка number автобусов перед их созданием (либо ловить exception уникальности) | ||
| 2. Содержимое rake таски я перевел в service. Это позволяет удобнее тестировать, а также возможность использовать в нескольких места, если такое потребуется. Плюс так удобнее запускать из консоли и дебажить | ||
|
|
||
| 3. Было создано два сервиса - Один работает в обычном варианте (берет и грузить все в память), другой в потоковом. |
| ImportTrups::ParseStreamJsonService.new(file_path: path_to_very_large_file).call | ||
| ``` | ||
| 9. Согласно рекомендациям по стилу rails ([rails style guide](https://github.com/arbox/rails-style-guide/blob/master/README-ruRU.md#has-many-through)) рекомендуют использовать `has_many :through`, но в данном учебном проекте я не стал это делать, хотя исключительно `has_many :through` использую в рабочих проектах | ||
| 10. Если посмотреть во view - то достаточно много partial`s, которые, вроде как, необязательно должны быть, так как не так много там html + неиспользуется в других местах. От partial's я тоже не стал избавляться. Надеюсь, не засчитают это минусом) |
There was a problem hiding this comment.
Подвох с паршлами в том, что они рендерятся циклом. Лучше их оставить и рендерить коллекцией.
|
|
||
|  | ||
|
|
||
| А работа с файлом 1M.json заняла чуть больше минуты |
| Также, удалось оптимизировать отображение самих рейсов в представлении | ||
|
|
||
| А также научился пользоваться PGHero, ощутил его мощь и пользу. | ||
| А еще узнал про потоковую запись и чтение в postresql и даже есть где это использовать в реальном проекте =) |
| disable_ddl_transaction! | ||
|
|
||
| def change | ||
| add_index(:buses, [:number, :model], unique: true, algorithm: :concurrently) |
There was a problem hiding this comment.
Плюсик за concurrently
В учебном проекте конечно не важно, а вот на проде полезно
| disable_ddl_transaction! | ||
|
|
||
| def change | ||
| add_index(:trips, [:from_id, :to_id], algorithm: :concurrently) |
There was a problem hiding this comment.
Плюсик за составной индекс 👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.